Skip to content

Commit

Permalink
Merge pull request #80 from skuid/PLIN-2287-improve-picard-filtering
Browse files Browse the repository at this point in the history
Fixes PLIN-2287 Improve Picard Filtering
plusplusben authored Aug 6, 2019
2 parents f911e12 + 3edb46f commit b679fe6
Showing 12 changed files with 426 additions and 213 deletions.
71 changes: 26 additions & 45 deletions delete.go
Original file line number Diff line number Diff line change
@@ -5,7 +5,6 @@ import (
"reflect"

sq "github.com/Masterminds/squirrel"
"github.com/skuid/picard/queryparts"
"github.com/skuid/picard/reflectutil"

"github.com/skuid/picard/query"
@@ -17,38 +16,46 @@ import (
// Returns the number of rows affected or an error.
func (porm PersistenceORM) DeleteModel(model interface{}) (int64, error) {

associations, err := getAssociationsFromModel(model)
metadata, err := tags.GetTableMetadata(model)
if err != nil {
return 0, err
}

tbl, err := query.Build(porm.multitenancyValue, model, queryparts.SelectFilter{}, nil)
hasAssociations, err := hasAssociations(model, metadata)
if err != nil {
return 0, err
}

pkField := metadata.GetPrimaryKeyFieldName()
pkColumn := metadata.GetPrimaryKeyColumnName()

tbl, err := query.Build(porm.multitenancyValue, model, nil, nil, nil, metadata)

if err != nil {
return 0, err
}

dSQL := tbl.DeleteSQL()

lookupPks := make([]interface{}, 0)
if len(associations) > 0 {
_, pk := reflectutil.ReflectTableInfo(reflect.TypeOf(model))
if hasAssociations {
results, err := porm.FilterModel(FilterRequest{
FilterModel: model,
Associations: associations,
SelectFields: []string{pkField},
})
if err != nil {
return 0, err
}

for _, result := range results {
val, ok := reflectutil.GetPK(reflect.ValueOf(result))
if ok {
val := getValueFromLookupString(reflect.ValueOf(result), pkField)
if val.IsValid() {
lookupPks = append(lookupPks, val.Interface())
}
}
dSQL = dSQL.Where(
sq.Eq{
fmt.Sprintf("%s.%s", tbl.Alias, pk): lookupPks,
fmt.Sprintf("%s.%s", tbl.Alias, pkColumn): lookupPks,
},
)
}
@@ -74,51 +81,25 @@ func (porm PersistenceORM) DeleteModel(model interface{}) (int64, error) {
return results.RowsAffected()
}

func getAssociationsFromModel(model interface{}) ([]tags.Association, error) {

func hasAssociations(model interface{}, metadata *tags.TableMetadata) (bool, error) {
val, err := stringutil.GetStructValue(model)

if err != nil {
return nil, err
return false, err
}

return getAssociationsFromValue(val)
}

func getAssociationsFromValue(val reflect.Value) ([]tags.Association, error) {
associations := make([]tags.Association, 0)

if val.Kind() != reflect.Struct {
return nil, fmt.Errorf("Model must be a struct in order to get associations. It was a %v instead", val.Kind())
return false, fmt.Errorf("Model must be a struct in order to get associations. It was a %v instead", val.Kind())
}

for i := 0; i < val.Type().NumField(); i++ {
structField := val.Type().Field(i)
ptags := tags.GetStructTagsMap(structField, "picard")

_, isFK := ptags["foreign_key"]

if isFK {
if relatedName, ok := ptags["related"]; ok {
for _, fkField := range metadata.GetForeignKeys() {
relatedName := fkField.RelatedFieldName

relatedVal := val.FieldByName(relatedName)

if !reflectutil.IsZeroValue(relatedVal) {
fieldAssoc := tags.Association{
Name: relatedName,
}

childAssocs, err := getAssociationsFromValue(relatedVal)
if err != nil {
return nil, err
}
fieldAssoc.Associations = append(fieldAssoc.Associations, childAssocs...)

associations = append(associations, fieldAssoc)
}
if relatedName != "" {
relatedVal := val.FieldByName(relatedName)
if !reflectutil.IsZeroValue(relatedVal) {
return true, nil
}
}
}

return associations, nil
return false, nil
}
18 changes: 3 additions & 15 deletions delete_test.go
Original file line number Diff line number Diff line change
@@ -43,8 +43,8 @@ func TestDeleteModel(t *testing.T) {
`)).
WithArgs(
"00000000-0000-0000-0000-000000000001",
"00000000-0000-0000-0000-000000000555",
).
"00000000-0000-0000-0000-000000000555",
).
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectCommit()
},
@@ -63,22 +63,10 @@ func TestDeleteModel(t *testing.T) {
mock.ExpectQuery(testdata.FmtSQLRegex(`
SELECT
t0.id AS "t0.id",
t0.organization_id AS "t0.organization_id",
t0.name AS "t0.name",
t0.nullable_lookup AS "t0.nullable_lookup",
t0.type AS "t0.type",
t0.is_active AS "t0.is_active",
t0.parent_id AS "t0.parent_id",
t0.config AS "t0.config",
t0.created_by_id AS "t0.created_by_id",
t0.updated_by_id AS "t0.updated_by_id",
t0.created_at AS "t0.created_at",
t0.updated_at AS "t0.updated_at",
t1.id AS "t1.id",
t1.organization_id AS "t1.organization_id",
t1.name AS "t1.name"
FROM testobject AS t0
LEFT JOIN parenttest AS t1 ON
JOIN parenttest AS t1 ON
(t1.id = t0.parent_id AND t1.organization_id = $1)
WHERE
t0.organization_id = $2 AND
21 changes: 12 additions & 9 deletions filter.go
Original file line number Diff line number Diff line change
@@ -8,18 +8,18 @@ import (
sq "github.com/Masterminds/squirrel"
"github.com/skuid/picard/query"
qp "github.com/skuid/picard/queryparts"
"github.com/skuid/picard/reflectutil"
"github.com/skuid/picard/stringutil"
"github.com/skuid/picard/tags"
)

// FilterRequest holds information about a request to filter on a model
type FilterRequest struct {
FilterModel interface{}
FieldFilters []qp.FieldFilter
Associations []tags.Association
OrderBy []qp.OrderByRequest
Runner sq.BaseRunner
Fields qp.SelectFilter
SelectFields []string
}

func addOrderBy(builder sq.SelectBuilder, orderBy []qp.OrderByRequest, filterMetadata *tags.TableMetadata, tableAlias string) sq.SelectBuilder {
@@ -39,7 +39,7 @@ func addOrderBy(builder sq.SelectBuilder, orderBy []qp.OrderByRequest, filterMet

func (p PersistenceORM) getSingleFilterResults(request FilterRequest, filterMetadata *tags.TableMetadata) ([]*reflect.Value, error) {
filterModel := request.FilterModel
tbl, err := query.Build(p.multitenancyValue, filterModel, request.Fields, request.Associations)
tbl, err := query.Build(p.multitenancyValue, filterModel, request.FieldFilters, request.Associations, request.SelectFields, filterMetadata)
if err != nil {
return nil, err
}
@@ -50,7 +50,7 @@ func (p PersistenceORM) getSingleFilterResults(request FilterRequest, filterMeta
return nil, err
}
aliasMap := tbl.FieldAliases()
return query.Hydrate(filterModel, aliasMap, rows)
return query.Hydrate(filterModel, aliasMap, rows, filterMetadata)
}

func (p PersistenceORM) getMultiFilterResults(request FilterRequest, filterMetadata *tags.TableMetadata) ([]*reflect.Value, error) {
@@ -67,7 +67,7 @@ func (p PersistenceORM) getMultiFilterResults(request FilterRequest, filterMetad
for i := 0; i < modelVal.Len(); i++ {
val := modelVal.Index(i)

ftbl, err := query.Build(mtVal, val.Interface(), request.Fields, request.Associations)
ftbl, err := query.Build(mtVal, val.Interface(), request.FieldFilters, request.Associations, request.SelectFields, filterMetadata)
if err != nil {
return nil, err
}
@@ -111,7 +111,7 @@ func (p PersistenceORM) getMultiFilterResults(request FilterRequest, filterMetad
return nil, err
}
aliasMap := tbl.FieldAliases()
return query.Hydrate(filterModel, aliasMap, rows)
return query.Hydrate(filterModel, aliasMap, rows, filterMetadata)
}

func (p PersistenceORM) getFilterResults(request FilterRequest, filterMetadata *tags.TableMetadata) ([]*reflect.Value, error) {
@@ -163,13 +163,14 @@ func (p PersistenceORM) FilterModel(request FilterRequest) ([]interface{}, error
if foreignKey != nil {
for _, result := range results {
newFilter := reflect.Indirect(reflect.New(childType))
pkval, ok := reflectutil.GetPK(*result)
if !ok {
pkval := getValueFromLookupString(*result, childMetadata.GetPrimaryKeyFieldName())

if !pkval.IsValid() {
return nil, fmt.Errorf("Missing 'primary_key' tag on type '%v'", result.Type().Name())
}

if fmf := newFilter.FieldByName(foreignKey.FieldName); fmf.CanSet() {
fmf.Set(*pkval)
fmf.Set(pkval)
} else {
return nil, fmt.Errorf("'foreign_key' field '%s' on 'child' type '%v' is not settable", foreignKey.FieldName, newFilter.Type())
}
@@ -207,6 +208,8 @@ func (p PersistenceORM) FilterModel(request FilterRequest) ([]interface{}, error
Associations: association.Associations,
OrderBy: association.OrderBy,
Runner: request.Runner,
FieldFilters: association.FieldFilters,
SelectFields: association.SelectFields,
})
if err != nil {
return nil, err
Loading

0 comments on commit b679fe6

Please sign in to comment.