Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Commit

Permalink
Fieldtype.ConvertToModelWithType (#2274)
Browse files Browse the repository at this point in the history 
# TL;DR

When changing the type of a work item we have to check if fields are compatible. This logic was deeply embedded in the work item repository but it can be outsourced and made available to other pieces of the code as well.

# About

`ConvertToModelWithType` tries to find way to convert the value `v` from the current `FieldType` to the other `FieldType` in model representation; returns `error` otherwise.

# Examples

* For example if the given value `v` is a `string` and the other `FieldType` is a string list, we will return the value `v` as an array of `interface{}` objects. 
* Let's say the current `FieldType` is a string list and the other `FieldType` is a single `string` field, then we check if the value `v` has only one element and return that instead of the whole list.
  • Loading branch information
@kwk
kwk authored Sep 5, 2018
1 parent 6774109 commit c0eaf33
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 156 deletions.
11 changes: 11 additions & 0 deletions workitem/field_definition.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ type FieldType interface {
// look at the implementation of this function to find out what's actually
// been checked for each individual type.
Validate() error
// ConvertToModelWithType tries to find way to convert the value v from this
// FieldType to the other FieldType in model representation; returns error
// otherwise.
//
// For example if the given value v is a string and the other FieldType is a
// string list, we will return the value v as an array of interfaces.
//
// Let's say the current FieldType is a string list and the other FieldType
// is a string field, then we check if the value v has only one element and
// return that instead of the whole list.
ConvertToModelWithType(other FieldType, v interface{}) (interface{}, error)
}

// FieldDefinition describes type & other restrictions of a field
Expand Down
34 changes: 34 additions & 0 deletions workitem/simple_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,37 @@ func (t SimpleType) ConvertFromModel(value interface{}) (interface{}, error) {
return nil, errs.Errorf("unexpected field type: %s", t.GetKind())
}
}

// ConvertToModelWithType implements FieldType
func (t SimpleType) ConvertToModelWithType(newFieldType FieldType, v interface{}) (interface{}, error) {
// Try to assign the old value to the new field
newVal, err := newFieldType.ConvertToModel(v)
if err == nil {
return newVal, nil
}

// if the new type is a list, stuff the old value in a list and
// try to assign it
if newFieldType.GetKind() == KindList {
newVal, err = newFieldType.ConvertToModel([]interface{}{v})
if err == nil {
return newVal, nil
}
}

// if the old type is a list but the new one isn't check that
// the list contains only one element and assign that
if t.GetKind() == KindList && newFieldType.GetKind() != KindList {
ifArr, ok := v.([]interface{})
if !ok {
return nil, errs.Errorf("failed to convert value to interface array: %+v", v)
}
if len(ifArr) == 1 {
newVal, err = newFieldType.ConvertToModel(ifArr[0])
if err == nil {
return newVal, nil
}
}
}
return nil, errs.Errorf("failed to convert value %+v (%[1]T) to field type %+v (%[2]T)", v, newFieldType)
}
142 changes: 142 additions & 0 deletions workitem/simple_type_blackbox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,145 @@ func TestSimpleType_SetDefaultValue(t *testing.T) {
})
}
}

type testData struct {
name string

initialValue interface{}
targetValue interface{}

initialFieldType FieldType
targetFieldType FieldType

fieldConvertible bool
}

func getFieldTypeConversionTestData() []testData {
k := KindString
return []testData{
// valid conversions
{"ok - simple type to simple type",
"foo1",
"foo1",
SimpleType{Kind: k},
SimpleType{Kind: k},
true},
{"ok - simple type to list",
"foo2",
[]interface{}{"foo2"},
SimpleType{Kind: k},
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
true},
{"ok - simple type to enum",
"foo3",
"foo3",
SimpleType{Kind: k},
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"red", "foo3", "blue"}},
true},
{"ok - list to list",
[]interface{}{"foo4", "foo5"},
[]interface{}{"foo4", "foo5"},
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
true},
{"ok - list to simple type",
[]interface{}{"foo6"},
"foo6",
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
SimpleType{Kind: k},
true},
{"ok - list to enum",
[]interface{}{"foo7"},
"foo7",
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"yellow", "foo7", "cyan"}},
true},
{"ok - enum to enum",
"foo8",
"foo8",
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"Bach", "foo8", "Chapdelaine"}},
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"Kant", "Hume", "foo8", "Aristoteles"}},
true},
{"ok - enum to simple type",
"foo9",
"foo9",
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"Schopenhauer", "foo9", "Duerer"}},
SimpleType{Kind: k},
true},
{"ok - enum to list",
"foo10",
[]interface{}{"foo10"},
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"Sokrates", "foo10", "Fromm"}},
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
true},
// invalid conversions
{"err - simple type (string) to simple type (int)",
"foo11",
nil,
SimpleType{Kind: KindString},
SimpleType{Kind: KindInteger},
false},
{"err - simple type (string) to list (integer)",
"foo2",
([]interface{})(nil),
SimpleType{Kind: k},
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: KindInteger}},
false},
{"err - simple type (string) to enum (float)",
"foo3",
11.1,
SimpleType{Kind: k},
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: KindFloat}, Values: []interface{}{11.1, 22.2, 33.3}},
false},
{"err - list (string) to list (float)",
[]interface{}{"foo4", "foo5"},
([]interface{})(nil),
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: KindFloat}},
false},
{"err - list (string) to simple type (int)",
[]interface{}{"foo6"},
nil,
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
SimpleType{Kind: KindInteger},
false},
{"err - list (string) to enum (float)",
[]interface{}{"foo7"},
11.1,
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: k}},
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: KindFloat}, Values: []interface{}{11.1, 22.2, 33.3}},
false},
{"err - enum (string) to enum (float)",
"foo8",
11.1,
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"Bach", "foo8", "Chapdelaine"}},
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: KindFloat}, Values: []interface{}{11.1, 22.2, 33.3}},
false},
{"err - enum (string) to simple type (float)",
"foo9",
nil,
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"Schopenhauer", "foo9", "Duerer"}},
SimpleType{Kind: KindFloat},
false},
{"err - enum (string) to list (float)",
"foo10",
([]interface{})(nil),
EnumType{SimpleType: SimpleType{Kind: KindEnum}, BaseType: SimpleType{Kind: k}, Values: []interface{}{"Sokrates", "foo10", "Fromm"}},
ListType{SimpleType: SimpleType{Kind: KindList}, ComponentType: SimpleType{Kind: KindFloat}},
false},
}
}

func TestConvertToModelWithType(t *testing.T) {
for _, d := range getFieldTypeConversionTestData() {
t.Run(d.name, func(t *testing.T) {
convertedVal, err := d.initialFieldType.ConvertToModelWithType(d.targetFieldType, d.initialValue)
if !d.fieldConvertible {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, convertedVal, d.targetValue)
})
}
}
32 changes: 4 additions & 28 deletions workitem/workitem_repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ func (r *GormWorkItemRepository) ChangeWorkItemType(ctx context.Context, wiStora
}
var fieldDiff = Fields{}
// Loop through old workitem type
for oldFieldName := range oldWIType.Fields {
for oldFieldName, oldFieldDef := range oldWIType.Fields {
// Temporary workaround to not add metastates to the field diff. We need
// to have a special handling for fields that shouldn't be set by user
// (or affected by type change) MetaState is a system level detail and
Expand All @@ -1197,33 +1197,9 @@ func (r *GormWorkItemRepository) ChangeWorkItemType(ctx context.Context, wiStora
}
// The field exists in old type and new type
if newField, ok := newWIType.Fields[oldFieldName]; ok {
// Try to assign the old value to the new field
_, err := newField.Type.ConvertToModel(wiStorage.Fields[oldFieldName])
if err != nil {
// if the new type is a list, stuff the old value in a list and
// try to assign it
if newField.Type.GetKind() == KindList {
var convertedValue interface{}
convertedValue, err = newField.Type.ConvertToModel([]interface{}{wiStorage.Fields[oldFieldName]})
if err == nil {
wiStorage.Fields[oldFieldName] = convertedValue
}
}
// if the old type is a list but the new one isn't check that
// the list contains only one element and assign that
if oldWIType.Fields[oldFieldName].Type.GetKind() == KindList && newField.Type.GetKind() != KindList {
ifArr, ok := wiStorage.Fields[oldFieldName].([]interface{})
if !ok {
return errs.Errorf("failed to convert field \"%s\" to interface array: %+v", oldFieldName, wiStorage.Fields[oldFieldName])
}
if len(ifArr) == 1 {
var convertedValue interface{}
convertedValue, err = newField.Type.ConvertToModel(ifArr[0])
if err == nil {
wiStorage.Fields[oldFieldName] = convertedValue
}
}
}
newVal, err := oldFieldDef.Type.ConvertToModelWithType(newField.Type, wiStorage.Fields[oldFieldName])
if err == nil {
wiStorage.Fields[oldFieldName] = newVal
}
// Failed to assign the old value to the new field. Add the field to
// the diff and remove it from the old workitem.
Expand Down
129 changes: 1 addition & 128 deletions workitem/workitem_repository_blackbox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,134 +145,7 @@ func (s *workItemRepoBlackBoxTest) TestSave() {
})

s.T().Run("type change", func(t *testing.T) {

type testData struct {
name string

initialValue interface{}
targetValue interface{}

initialFieldType workitem.FieldType
targetFieldType workitem.FieldType

fieldConvertible bool
}

k := workitem.KindString

td := []testData{
// valid conversions
{"ok - simple type to simple type",
"foo1",
"foo1",
workitem.SimpleType{Kind: k},
workitem.SimpleType{Kind: k},
true},
{"ok - simple type to list",
"foo2",
[]interface{}{"foo2"},
workitem.SimpleType{Kind: k},
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
true},
{"ok - simple type to enum",
"foo3",
"foo3",
workitem.SimpleType{Kind: k},
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"red", "foo3", "blue"}},
true},
{"ok - list to list",
[]interface{}{"foo4", "foo5"},
[]interface{}{"foo4", "foo5"},
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
true},
{"ok - list to simple type",
[]interface{}{"foo6"},
"foo6",
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
workitem.SimpleType{Kind: k},
true},
{"ok - list to enum",
[]interface{}{"foo7"},
"foo7",
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"yellow", "foo7", "cyan"}},
true},
{"ok - enum to enum",
"foo8",
"foo8",
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"Bach", "foo8", "Chapdelaine"}},
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"Kant", "Hume", "foo8", "Aristoteles"}},
true},
{"ok - enum to simple type",
"foo9",
"foo9",
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"Schopenhauer", "foo9", "Duerer"}},
workitem.SimpleType{Kind: k},
true},
{"ok - enum to list",
"foo10",
[]interface{}{"foo10"},
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"Sokrates", "foo10", "Fromm"}},
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
true},
// invalid conversions
{"err - simple type (string) to simple type (int)",
"foo11",
nil,
workitem.SimpleType{Kind: workitem.KindString},
workitem.SimpleType{Kind: workitem.KindInteger},
false},
{"err - simple type (string) to list (integer)",
"foo2",
([]interface{})(nil),
workitem.SimpleType{Kind: k},
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: workitem.KindInteger}},
false},
{"err - simple type (string) to enum (float)",
"foo3",
11.1,
workitem.SimpleType{Kind: k},
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: workitem.KindFloat}, Values: []interface{}{11.1, 22.2, 33.3}},
false},
{"err - list (string) to list (float)",
[]interface{}{"foo4", "foo5"},
([]interface{})(nil),
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: workitem.KindFloat}},
false},
{"err - list (string) to simple type (int)",
[]interface{}{"foo6"},
nil,
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
workitem.SimpleType{Kind: workitem.KindInteger},
false},
{"err - list (string) to enum (float)",
[]interface{}{"foo7"},
11.1,
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: k}},
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: workitem.KindFloat}, Values: []interface{}{11.1, 22.2, 33.3}},
false},
{"err - enum (string) to enum (float)",
"foo8",
11.1,
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"Bach", "foo8", "Chapdelaine"}},
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: workitem.KindFloat}, Values: []interface{}{11.1, 22.2, 33.3}},
false},
{"err - enum (string) to simple type (float)",
"foo9",
nil,
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"Schopenhauer", "foo9", "Duerer"}},
workitem.SimpleType{Kind: workitem.KindFloat},
false},
{"err - enum (string) to list (float)",
"foo10",
([]interface{})(nil),
workitem.EnumType{SimpleType: workitem.SimpleType{Kind: workitem.KindEnum}, BaseType: workitem.SimpleType{Kind: k}, Values: []interface{}{"Sokrates", "foo10", "Fromm"}},
workitem.ListType{SimpleType: workitem.SimpleType{Kind: workitem.KindList}, ComponentType: workitem.SimpleType{Kind: workitem.KindFloat}},
false},
}
for _, d := range td {
for _, d := range getFieldTypeConversionTestData() {
t.Run(d.name, func(t *testing.T) {
fieldName := d.name

Expand Down

0 comments on commit c0eaf33

Please sign in to comment.