From 1457bc40322c6488c0e04c20f1a3fe101ddbac82 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Wed, 2 Jan 2019 18:37:45 +0100 Subject: [PATCH] Optimize module, page loading --- codegen.sh | 2 +- crm/repository/module.go | 81 +++++++++++++++++++++++++++----------- crm/repository/record.go | 36 ++++++----------- crm/rest/module.go | 2 +- crm/service/module.go | 22 +---------- crm/service/module_test.go | 12 +++--- crm/service/record.go | 38 +++++++++++------- crm/service/record_test.go | 25 ++++++------ crm/service/record_util.go | 33 ++++++++++++---- crm/types/type.gen.go | 35 ++++++++++++++++ crm/types/types.go | 30 +++++++++++--- 11 files changed, 199 insertions(+), 117 deletions(-) diff --git a/codegen.sh b/codegen.sh index 70b490949..ed793293d 100755 --- a/codegen.sh +++ b/codegen.sh @@ -37,7 +37,7 @@ function types { CGO_ENABLED=0 go build -o ./build/gen-type-set codegen/v2/type-set.go fi - ./build/gen-type-set --types Module,Page,Chart --output crm/types/type.gen.go + ./build/gen-type-set --types Module,Page,Chart -no-pk-types ModuleField --output crm/types/type.gen.go ./build/gen-type-set --types MessageAttachment --output sam/types/attachment.gen.go ./build/gen-type-set --types Channel --output sam/types/channel.gen.go diff --git a/crm/repository/module.go b/crm/repository/module.go index 58c675c17..de7807e0d 100644 --- a/crm/repository/module.go +++ b/crm/repository/module.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/jmoiron/sqlx" "github.com/pkg/errors" "github.com/titpetric/factory" @@ -19,9 +20,6 @@ type ( Create(mod *types.Module) (*types.Module, error) Update(mod *types.Module) (*types.Module, error) DeleteByID(id uint64) error - - Fields(mod *types.Module) (types.ModuleFieldSet, error) - FieldNames(mod *types.Module) ([]string, error) } module struct { @@ -29,6 +27,16 @@ type ( } ) +const ( + sqlModuleColumns = ` + id, name, json, + created_at, updated_at, deleted_at + ` + sqlModuleSelect = ` + SELECT ` + sqlModuleColumns + ` FROM crm_module WHERE deleted_at IS NULL + ` +) + func Module(ctx context.Context, db *factory.DB) ModuleRepository { return (&module{}).With(ctx, db) } @@ -41,17 +49,36 @@ func (r *module) With(ctx context.Context, db *factory.DB) ModuleRepository { // @todo: update to accepted DeletedAt column semantics from SAM -func (r *module) FindByID(id uint64) (*types.Module, error) { - mod := &types.Module{} - if err := r.db().Get(mod, "SELECT * FROM crm_module WHERE id=?", id); err != nil { - return nil, err +func (r *module) FindByID(id uint64) (mod *types.Module, err error) { + mod = &types.Module{} + + if err = r.db().Get(mod, sqlModuleSelect+" AND id = ? ", id); err != nil { + return } - return mod, nil + + if mod.Fields, err = r.fields(id); err != nil { + return + } + + return } -func (r *module) Find() (types.ModuleSet, error) { - mod := types.ModuleSet{} - return mod, r.db().Select(&mod, "SELECT * FROM crm_module ORDER BY id ASC") +func (r *module) Find() (mm types.ModuleSet, err error) { + if err = r.db().Select(&mm, sqlModuleSelect+" ORDER BY id ASC"); err != nil { + return + } + + var ff types.ModuleFieldSet + if ff, err = r.fields(mm.IDs()...); err != nil { + return + } else { + _ = ff.Walk(func(f *types.ModuleField) error { + mm.FindByID(f.ModuleID).Fields = append(mm.FindByID(f.ModuleID).Fields, f) + return nil + }) + } + + return mm, nil } func (r *module) Create(mod *types.Module) (*types.Module, error) { @@ -98,19 +125,27 @@ func (r *module) DeleteByID(id uint64) error { return err } -func (r *module) Fields(mod *types.Module) (ff types.ModuleFieldSet, err error) { - return ff, r.db().Select(&ff, "select * from crm_module_form where module_id=? order by place asc", mod.ID) -} +func (r *module) fields(IDs ...uint64) (ff types.ModuleFieldSet, err error) { + if len(IDs) == 0 { + return + } -// FieldNames returns a slice of field names, used for ordering record row columns -func (r *module) FieldNames(mod *types.Module) ([]string, error) { - if fields, err := r.Fields(mod); err != nil { - return []string{}, err + if sql, args, err := sqlx.In("SELECT * FROM crm_module_form WHERE module_id IN (?) ORDER BY module_id AND place", IDs); err != nil { + return nil, err } else { - result := make([]string, len(fields)) - for k, v := range fields { - result[k] = v.Name - } - return result, nil + return ff, r.db().Select(&ff, sql, args...) } } + +// // FieldNames returns a slice of field names, used for ordering record row columns +// func (r *module) FieldNames(mod *types.Module) ([]string, error) { +// if fields, err := r.Fields(mod.ID); err != nil { +// return []string{}, err +// } else { +// result := make([]string, len(fields)) +// for k, v := range fields { +// result[k] = v.Name +// } +// return result, nil +// } +// } diff --git a/crm/repository/record.go b/crm/repository/record.go index dffb9b1ef..4c7bbcc83 100644 --- a/crm/repository/record.go +++ b/crm/repository/record.go @@ -22,13 +22,13 @@ type ( FindByID(id uint64) (*types.Record, error) Report(moduleID uint64, metrics, dimensions, filter string) (results interface{}, err error) - Find(moduleID uint64, filter string, sort string, page int, perPage int) (*FindResponse, error) + Find(module *types.Module, filter string, sort string, page int, perPage int) (*FindResponse, error) Create(mod *types.Record) (*types.Record, error) Update(mod *types.Record) (*types.Record, error) DeleteByID(id uint64) error - Fields(mod *types.Record) ([]*types.RecordColumn, error) + Fields(module *types.Module, record *types.Record) ([]*types.RecordColumn, error) } FindResponseMeta struct { @@ -103,7 +103,7 @@ func (r *record) Report(moduleID uint64, metrics, dimensions, filter string) (re } } -func (r *record) Find(moduleID uint64, filter string, sort string, page int, perPage int) (*FindResponse, error) { +func (r *record) Find(module *types.Module, filter string, sort string, page int, perPage int) (*FindResponse, error) { if page < 0 { page = 0 } @@ -116,6 +116,7 @@ func (r *record) Find(moduleID uint64, filter string, sort string, page int, per if perPage < 10 { perPage = 10 } + response := &FindResponse{ Meta: FindResponseMeta{ Filter: filter, @@ -130,7 +131,7 @@ func (r *record) Find(moduleID uint64, filter string, sort string, page int, per query := squirrel. Select(). From("crm_record"). - Where("(module_id = ? AND deleted_at IS NULL AND json IS NOT NULL)", moduleID) + Where("(module_id = ? AND deleted_at IS NULL AND json IS NOT NULL)", module.ID) // Parse filters. p := ql.NewParser() @@ -170,18 +171,9 @@ func (r *record) Find(moduleID uint64, filter string, sort string, page int, per // Append Sorting. chuncks := strings.Split(sort, ",") if len(chuncks) > 0 { - - // Ger module fields. - modulRepo := Module(r.Context(), r.db()) - mod, err := modulRepo.FindByID(moduleID) - if err != nil { - return nil, err - } - modFields, err := modulRepo.FieldNames(mod) - if err != nil { - return nil, err - } fieldMap := make(map[string]bool) + modFields := module.Fields.Names() + for i := 0; i < len(modFields); i++ { fieldMap[modFields[i]] = true } @@ -334,19 +326,15 @@ func (r *record) DeleteByID(id uint64) error { return err } -func (r *record) Fields(record *types.Record) ([]*types.RecordColumn, error) { +func (r *record) Fields(module *types.Module, record *types.Record) ([]*types.RecordColumn, error) { result := make([]*types.RecordColumn, 0) - module := Module(r.ctx, r.db()) - mod, err := module.FindByID(record.ModuleID) - if err != nil { - return result, err + if module.ID != record.ModuleID { + return result, errors.New("Record does not belong to the module") } - fieldNames, err := module.FieldNames(mod) - if err != nil { - return result, err - } + fieldNames := module.Fields.Names() + if len(fieldNames) == 0 { return result, errors.New("Module has no fields") } diff --git a/crm/rest/module.go b/crm/rest/module.go index 6a88cc171..4e334c90d 100644 --- a/crm/rest/module.go +++ b/crm/rest/module.go @@ -64,7 +64,7 @@ func (s *Module) RecordList(ctx context.Context, r *request.ModuleRecordList) (i } func (s *Module) RecordRead(ctx context.Context, r *request.ModuleRecordRead) (interface{}, error) { - return s.content.With(ctx).FindByID(r.RecordID) + return s.content.With(ctx).FindByID(r.ModuleID, r.RecordID) } func (s *Module) RecordCreate(ctx context.Context, r *request.ModuleRecordCreate) (interface{}, error) { diff --git a/crm/service/module.go b/crm/service/module.go index b6072460d..35a8ecf0c 100644 --- a/crm/service/module.go +++ b/crm/service/module.go @@ -28,8 +28,6 @@ type ( Create(module *types.Module) (*types.Module, error) Update(module *types.Module) (*types.Module, error) DeleteByID(moduleID uint64) error - - FieldNames(mod *types.Module) ([]string, error) } ) @@ -52,9 +50,7 @@ func (s *module) FindByID(id uint64) (*types.Module, error) { if err != nil { return nil, err } - if err := s.preload(mod); err != nil { - return nil, err - } + return mod, err } @@ -99,19 +95,3 @@ func (s *module) Update(module *types.Module) (m *types.Module, err error) { func (s *module) DeleteByID(id uint64) error { return s.moduleRepo.DeleteByID(id) } - -func (s *module) FieldNames(mod *types.Module) ([]string, error) { - return s.moduleRepo.FieldNames(mod) -} - -func (s *module) preload(mod *types.Module) (err error) { - if mod.Page, err = s.pageRepo.FindByModuleID(mod.ID); err != nil { - return - } - - if mod.Fields, err = s.moduleRepo.Fields(mod); err != nil { - return - } - - return -} diff --git a/crm/service/module_test.go b/crm/service/module_test.go index 21cb792dc..ef0aea63f 100644 --- a/crm/service/module_test.go +++ b/crm/service/module_test.go @@ -13,20 +13,20 @@ func TestModule(t *testing.T) { // the module object we're working with module := &types.Module{ Name: "Test", - Fields: []types.ModuleField{ - types.ModuleField{ + Fields: types.ModuleFieldSet{ + &types.ModuleField{ Name: "name", Kind: "input", }, - types.ModuleField{ + &types.ModuleField{ Name: "email", Kind: "email", }, - types.ModuleField{ + &types.ModuleField{ Name: "options", Kind: "select_multi", }, - types.ModuleField{ + &types.ModuleField{ Name: "description", Kind: "text", }, @@ -65,7 +65,7 @@ func TestModule(t *testing.T) { // fetch module fields { - fl, err := repository.FieldNames(m) + fl := m.Fields.Names() assert(t, err == nil, "Error when retrieving module fields by module: %+v", err) assert(t, len(fl) == 4, "Expected 4 fields, got %d", len(fl)) } diff --git a/crm/service/record.go b/crm/service/record.go index 27659f4f9..65b25fa18 100644 --- a/crm/service/record.go +++ b/crm/service/record.go @@ -19,6 +19,7 @@ type ( repository repository.RecordRepository pageRepo repository.PageRepository + moduleRepo repository.ModuleRepository userSvc systemService.UserService } @@ -26,7 +27,7 @@ type ( RecordService interface { With(ctx context.Context) RecordService - FindByID(recordID uint64) (*types.Record, error) + FindByID(moduleID uint64, recordID uint64) (*types.Record, error) Report(moduleID uint64, metrics, dimensions, filter string) (interface{}, error) Find(moduleID uint64, filter string, sort string, page int, perPage int) (*repository.FindResponse, error) @@ -35,7 +36,7 @@ type ( Update(record *types.Record) (*types.Record, error) DeleteByID(recordID uint64) error - Fields(mod *types.Record) ([]*types.RecordColumn, error) + Fields(module *types.Module, record *types.Record) ([]*types.RecordColumn, error) } ) @@ -52,30 +53,39 @@ func (s *record) With(ctx context.Context) RecordService { ctx: ctx, repository: repository.Record(ctx, db), pageRepo: repository.Page(ctx, db), + moduleRepo: repository.Module(ctx, db), userSvc: s.userSvc.With(ctx), } } -func (s *record) FindByID(id uint64) (*types.Record, error) { - response, err := s.repository.FindByID(id) - if err != nil { +func (s *record) FindByID(moduleID uint64, id uint64) (response *types.Record, err error) { + var module *types.Module + + if module, err = s.moduleRepo.FindByID(moduleID); err != nil { return nil, err } - return response, s.preload(response, "page", "user", "fields") + + if response, err = s.repository.FindByID(id); err != nil { + return nil, err + } + return response, s.preload(module, response, "page", "user", "fields") } func (s *record) Report(moduleID uint64, metrics, dimensions, filter string) (interface{}, error) { return s.repository.Report(moduleID, metrics, dimensions, filter) } -func (s *record) Find(moduleID uint64, filter string, sort string, page int, perPage int) (*repository.FindResponse, error) { - response, err := s.repository.Find(moduleID, filter, sort, page, perPage) - if err != nil { +func (s *record) Find(moduleID uint64, filter string, sort string, page int, perPage int) (response *repository.FindResponse, err error) { + var module *types.Module + + if module, err = s.moduleRepo.FindByID(moduleID); err != nil { return nil, err - } - if err := s.preloadAll(response.Records, "user", "fields"); err != nil { + } else if response, err = s.repository.Find(module, filter, sort, page, perPage); err != nil { + return nil, err + } else if err := s.preloadAll(module, response.Records, "user", "fields"); err != nil { return nil, err } + return response, nil } @@ -84,7 +94,7 @@ func (s *record) Create(mod *types.Record) (*types.Record, error) { if err != nil { return nil, err } - return response, s.preload(response, "user", "fields") + return response, s.preload(nil, response, "user", "fields") } func (s *record) Update(record *types.Record) (c *types.Record, err error) { @@ -110,8 +120,8 @@ func (s *record) Update(record *types.Record) (c *types.Record, err error) { }) } -func (s *record) Fields(mod *types.Record) ([]*types.RecordColumn, error) { - return s.repository.Fields(mod) +func (s *record) Fields(module *types.Module, record *types.Record) ([]*types.RecordColumn, error) { + return s.repository.Fields(module, record) } func (s *record) DeleteByID(id uint64) error { diff --git a/crm/service/record_test.go b/crm/service/record_test.go index 8ceade5a4..38bef3631 100644 --- a/crm/service/record_test.go +++ b/crm/service/record_test.go @@ -35,20 +35,20 @@ func TestRecord(t *testing.T) { module := &types.Module{ Name: "Test", - Fields: []types.ModuleField{ - types.ModuleField{ + Fields: types.ModuleFieldSet{ + &types.ModuleField{ Name: "name", Kind: "input", }, - types.ModuleField{ + &types.ModuleField{ Name: "email", Kind: "email", }, - types.ModuleField{ + &types.ModuleField{ Name: "options", Kind: "select_multi", }, - types.ModuleField{ + &types.ModuleField{ Name: "description", Kind: "text", }, @@ -56,11 +56,10 @@ func TestRecord(t *testing.T) { } // set up a module - { - _, err := Module().With(context.Background()).Create(module) - assert(t, err == nil, "Error when creating module: %+v", err) - assert(t, module.ID > 0, "Expected auto generated ID") - } + var err error + module, err = Module().With(context.Background()).Create(module) + assert(t, err == nil, "Error when creating module: %+v", err) + assert(t, module.ID > 0, "Expected auto generated ID") columns := []types.RecordColumn{ types.RecordColumn{ @@ -140,13 +139,13 @@ func TestRecord(t *testing.T) { // fetch created record { - ms, err := repository.FindByID(m1.ID) + ms, err := repository.FindByID(module.ID, m1.ID) assert(t, err == nil, "Error when retrieving record by id: %+v", err) assert(t, ms.ID == m1.ID, "Expected ID from database to match, %d != %d", m1.ID, ms.ID) assert(t, ms.ModuleID == m1.ModuleID, "Expected Module ID from database to match, %d != %d", m1.ModuleID, ms.ModuleID) { - fields, err := repository.Fields(ms) + fields, err := repository.Fields(module, ms) // fields := make([]testRecordRow, 0) // err = json.Unmarshal(ms.Fields, &fields) assert(t, err == nil, "%+v", errors.Wrap(err, "Didn't expect error when unmarshalling")) @@ -174,7 +173,7 @@ func TestRecord(t *testing.T) { // re-fetch record { - ms, err := repository.FindByID(m1.ID) + ms, err := repository.FindByID(module.ID, m1.ID) assert(t, err == nil, "Error when retrieving record by id: %+v", err) assert(t, ms.ID == m1.ID, "Expected ID from database to match, %d != %d", m1.ID, ms.ID) assert(t, ms.ModuleID == m1.ModuleID, "Expected ID from database to match, %d != %d", m1.ModuleID, ms.ModuleID) diff --git a/crm/service/record_util.go b/crm/service/record_util.go index 944115d3d..710765cf1 100644 --- a/crm/service/record_util.go +++ b/crm/service/record_util.go @@ -6,20 +6,37 @@ import ( "github.com/crusttech/crust/crm/types" ) -func (r *record) preloadAll(records []*types.Record, fields ...string) error { +func (s *record) preloadAll(module *types.Module, records []*types.Record, fields ...string) (err error) { + if len(records) == 0 { + return nil + } + + if module == nil { + if module, err = s.moduleRepo.FindByID(records[0].ID); err != nil { + // Assuming all records are from the same module + return + } + } + for _, record := range records { - if err := r.preload(record, fields...); err != nil { + if err = s.preload(module, record, fields...); err != nil { + return + } + } + return +} + +func (s *record) preload(module *types.Module, record *types.Record, fields ...string) (err error) { + if module == nil { + if module, err = s.moduleRepo.FindByID(record.ModuleID); err != nil { return err } } - return nil -} -func (r *record) preload(record *types.Record, fields ...string) (err error) { for _, field := range fields { switch field { case "fields": - fields, err := r.Fields(record) + fields, err := s.Fields(module, record) if err != nil { return err } @@ -31,12 +48,12 @@ func (r *record) preload(record *types.Record, fields ...string) (err error) { return err } case "page": - if record.Page, err = r.pageRepo.FindByModuleID(record.ModuleID); err != nil { + if record.Page, err = s.pageRepo.FindByModuleID(record.ModuleID); err != nil { return } case "user": if record.UserID > 0 { - if record.User, err = r.userSvc.FindByID(record.UserID); err != nil { + if record.User, err = s.userSvc.FindByID(record.UserID); err != nil { return } } diff --git a/crm/types/type.gen.go b/crm/types/type.gen.go index 95dc95f67..b4e790f61 100644 --- a/crm/types/type.gen.go +++ b/crm/types/type.gen.go @@ -18,6 +18,11 @@ type ( // // This type is auto-generated. ChartSet []*Chart + + // ModuleFieldSet slice of ModuleField + // + // This type is auto-generated. + ModuleFieldSet []*ModuleField ) // Walk iterates through every slice item and calls w(Module) err @@ -187,3 +192,33 @@ func (set ChartSet) IDs() (IDs []uint64) { return } + +// Walk iterates through every slice item and calls w(ModuleField) err +// +// This function is auto-generated. +func (set ModuleFieldSet) Walk(w func(*ModuleField) error) (err error) { + for i := range set { + if err = w(set[i]); err != nil { + return + } + } + + return +} + +// Filter iterates through every slice item, calls f(ModuleField) (bool, err) and return filtered slice +// +// This function is auto-generated. +func (set ModuleFieldSet) Filter(f func(*ModuleField) (bool, error)) (out ModuleFieldSet, err error) { + var ok bool + out = ModuleFieldSet{} + for i := range set { + if ok, err = f(set[i]); err != nil { + return + } else if ok { + out = append(out, set[i]) + } + } + + return +} diff --git a/crm/types/types.go b/crm/types/types.go index b37cb05ed..b4914a6d2 100644 --- a/crm/types/types.go +++ b/crm/types/types.go @@ -77,8 +77,6 @@ type ( Visible bool `json:"isVisible" db:"is_visible"` } - ModuleFieldSet []ModuleField - // Page - page structure Page struct { ID uint64 `json:"pageID,string" db:"id"` @@ -111,13 +109,33 @@ type ( } ) -func (f *ModuleFieldSet) Scan(src interface{}) error { +func (set *ModuleFieldSet) Scan(src interface{}) error { if data, ok := src.([]byte); ok { - return json.Unmarshal(data, f) + return json.Unmarshal(data, set) } return nil } -func (f ModuleFieldSet) Value() (driver.Value, error) { - return json.Marshal(f) +func (set ModuleFieldSet) Value() (driver.Value, error) { + return json.Marshal(set) +} + +func (set ModuleFieldSet) Names() (names []string) { + names = make([]string, len(set)) + + for i := range set { + names[i] = set[i].Name + } + + return +} + +func (set ModuleFieldSet) FilterByModule(moduleID uint64) (ff ModuleFieldSet) { + for i := range set { + if set[i].ModuleID == moduleID { + ff = append(ff, set[i]) + } + } + + return }