From 44e35c9f5ae42902ed0c7e8bc7d7d660b74c9a99 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Thu, 25 Jan 2018 23:30:02 +0100 Subject: [PATCH 1/4] Remove unused selecting via int --- accessors.go | 128 +++++++++++++++------------------------------- accessors_test.go | 72 -------------------------- 2 files changed, 41 insertions(+), 159 deletions(-) diff --git a/accessors.go b/accessors.go index 204356a..41f9031 100644 --- a/accessors.go +++ b/accessors.go @@ -48,101 +48,55 @@ func (m Map) Set(selector string, value interface{}) Map { // access accesses the object using the selector and performs the // appropriate action. -func access(current, selector, value interface{}, isSet bool) interface{} { - switch selector.(type) { - case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64: - if array, ok := current.([]interface{}); ok { - index := intFromInterface(selector) - if index >= len(array) { - return nil - } - return array[index] - } - return nil +func access(current interface{}, selector string, value interface{}, isSet bool) interface{} { + selSegs := strings.SplitN(selector, PathSeparator, 2) + thisSel := selSegs[0] + index := -1 + var err error - case string: - selStr := selector.(string) - selSegs := strings.SplitN(selStr, PathSeparator, 2) - thisSel := selSegs[0] - index := -1 - var err error + if strings.Contains(thisSel, "[") { + arrayMatches := arrayAccesRegex.FindStringSubmatch(thisSel) + if len(arrayMatches) > 0 { + // Get the key into the map + thisSel = arrayMatches[1] - if strings.Contains(thisSel, "[") { - arrayMatches := arrayAccesRegex.FindStringSubmatch(thisSel) - if len(arrayMatches) > 0 { - // Get the key into the map - thisSel = arrayMatches[1] + // Get the index into the array at the key + index, err = strconv.Atoi(arrayMatches[2]) - // Get the index into the array at the key - index, err = strconv.Atoi(arrayMatches[2]) - - if err != nil { - // This should never happen. If it does, something has gone - // seriously wrong. Panic. - panic("objx: Array index is not an integer. Must use array[int].") - } + if err != nil { + // This should never happen. If it does, something has gone + // seriously wrong. Panic. + panic("objx: Array index is not an integer. Must use array[int].") } } - if curMap, ok := current.(Map); ok { - current = map[string]interface{}(curMap) - } - // get the object in question - switch current.(type) { - case map[string]interface{}: - curMSI := current.(map[string]interface{}) - if len(selSegs) <= 1 && isSet { - curMSI[thisSel] = value - return nil - } - current = curMSI[thisSel] - default: - current = nil + } + if curMap, ok := current.(Map); ok { + current = map[string]interface{}(curMap) + } + // get the object in question + switch current.(type) { + case map[string]interface{}: + curMSI := current.(map[string]interface{}) + if len(selSegs) <= 1 && isSet { + curMSI[thisSel] = value + return nil } - // do we need to access the item of an array? - if index > -1 { - if array, ok := current.([]interface{}); ok { - if index < len(array) { - current = array[index] - } else { - current = nil - } + current = curMSI[thisSel] + default: + current = nil + } + // do we need to access the item of an array? + if index > -1 { + if array, ok := current.([]interface{}); ok { + if index < len(array) { + current = array[index] + } else { + current = nil } } - if len(selSegs) > 1 { - current = access(current, selSegs[1], value, isSet) - } } - return current -} - -// intFromInterface converts an interface object to the largest -// representation of an unsigned integer using a type switch and -// assertions -func intFromInterface(selector interface{}) int { - var value int - switch selector.(type) { - case int: - value = selector.(int) - case int8: - value = int(selector.(int8)) - case int16: - value = int(selector.(int16)) - case int32: - value = int(selector.(int32)) - case int64: - value = int(selector.(int64)) - case uint: - value = int(selector.(uint)) - case uint8: - value = int(selector.(uint8)) - case uint16: - value = int(selector.(uint16)) - case uint32: - value = int(selector.(uint32)) - case uint64: - value = int(selector.(uint64)) - default: - return 0 + if len(selSegs) > 1 { + current = access(current, selSegs[1], value, isSet) } - return value + return current } diff --git a/accessors_test.go b/accessors_test.go index f6be310..a3968f2 100644 --- a/accessors_test.go +++ b/accessors_test.go @@ -66,78 +66,6 @@ func TestAccessorsAccessGetInsideArray(t *testing.T) { assert.Nil(t, current.Get("names[2]").Data()) } -func TestAccessorsAccessGetFromArrayWithInt(t *testing.T) { - current := []interface{}{ - map[string]interface{}{ - "first": "Tyler", - "last": "Bunnell", - }, - map[string]interface{}{ - "first": "Capitol", - "last": "Bollocks", - }, - } - one := access(current, 0, nil, false) - two := access(current, 1, nil, false) - three := access(current, 2, nil, false) - - assert.Equal(t, "Tyler", one.(map[string]interface{})["first"]) - assert.Equal(t, "Capitol", two.(map[string]interface{})["first"]) - assert.Nil(t, three) -} - -func TestAccessorsAccessGetFromArrayWithIntTypes(t *testing.T) { - current := []interface{}{ - "abc", - "def", - } - assert.Equal(t, "abc", access(current, 0, nil, false)) - assert.Equal(t, "def", access(current, 1, nil, false)) - assert.Nil(t, access(current, 2, nil, false)) - - assert.Equal(t, "abc", access(current, int8(0), nil, false)) - assert.Equal(t, "def", access(current, int8(1), nil, false)) - assert.Nil(t, access(current, int8(2), nil, false)) - - assert.Equal(t, "abc", access(current, int16(0), nil, false)) - assert.Equal(t, "def", access(current, int16(1), nil, false)) - assert.Nil(t, access(current, int16(2), nil, false)) - - assert.Equal(t, "abc", access(current, int32(0), nil, false)) - assert.Equal(t, "def", access(current, int32(1), nil, false)) - assert.Nil(t, access(current, int32(2), nil, false)) - - assert.Equal(t, "abc", access(current, int64(0), nil, false)) - assert.Equal(t, "def", access(current, int64(1), nil, false)) - assert.Nil(t, access(current, int64(2), nil, false)) - - assert.Equal(t, "abc", access(current, uint(0), nil, false)) - assert.Equal(t, "def", access(current, uint(1), nil, false)) - assert.Nil(t, access(current, uint(2), nil, false)) - - assert.Equal(t, "abc", access(current, uint8(0), nil, false)) - assert.Equal(t, "def", access(current, uint8(1), nil, false)) - assert.Nil(t, access(current, uint8(2), nil, false)) - - assert.Equal(t, "abc", access(current, uint16(0), nil, false)) - assert.Equal(t, "def", access(current, uint16(1), nil, false)) - assert.Nil(t, access(current, uint16(2), nil, false)) - - assert.Equal(t, "abc", access(current, uint32(0), nil, false)) - assert.Equal(t, "def", access(current, uint32(1), nil, false)) - assert.Nil(t, access(current, uint32(2), nil, false)) - - assert.Equal(t, "abc", access(current, uint64(0), nil, false)) - assert.Equal(t, "def", access(current, uint64(1), nil, false)) - assert.Nil(t, access(current, uint64(2), nil, false)) -} - -func TestAccessorsAccessGetFromArrayWithIntError(t *testing.T) { - current := Map{"name": "Tyler"} - - assert.Nil(t, access(current, 0, nil, false)) -} - func TestAccessorsGet(t *testing.T) { current := Map{"name": "Tyler"} From 8c1cb5005c048719d4e0dc0a203077924ff0b801 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Thu, 25 Jan 2018 23:32:30 +0100 Subject: [PATCH 2/4] Move test to package objx_test --- accessors_test.go | 126 +++++++++++++++++++++++----------------------- 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/accessors_test.go b/accessors_test.go index a3968f2..1bd2133 100644 --- a/accessors_test.go +++ b/accessors_test.go @@ -1,166 +1,168 @@ -package objx +package objx_test import ( "testing" + "github.com/stretchr/objx" "github.com/stretchr/testify/assert" ) func TestAccessorsAccessGetSingleField(t *testing.T) { - current := Map{"name": "Tyler"} + m := objx.Map{"name": "Tyler"} - assert.Equal(t, "Tyler", current.Get("name").Data()) + assert.Equal(t, "Tyler", m.Get("name").Data()) } func TestAccessorsAccessGetSingleFieldInt(t *testing.T) { - current := Map{"name": 10} + m := objx.Map{"name": 10} - assert.Equal(t, 10, current.Get("name").Data()) + assert.Equal(t, 10, m.Get("name").Data()) } func TestAccessorsAccessGetDeep(t *testing.T) { - current := Map{ - "name": Map{ + m := objx.Map{ + "name": objx.Map{ "first": "Tyler", "last": "Bunnell", }, } - assert.Equal(t, "Tyler", current.Get("name.first").Data()) - assert.Equal(t, "Bunnell", current.Get("name.last").Data()) + assert.Equal(t, "Tyler", m.Get("name.first").Data()) + assert.Equal(t, "Bunnell", m.Get("name.last").Data()) } func TestAccessorsAccessGetDeepDeep(t *testing.T) { - current := Map{ - "one": Map{ - "two": Map{ - "three": Map{ + m := objx.Map{ + "one": objx.Map{ + "two": objx.Map{ + "three": objx.Map{ "four": 4, }, }, }, } - assert.Equal(t, 4, current.Get("one.two.three.four").Data()) + assert.Equal(t, 4, m.Get("one.two.three.four").Data()) } func TestAccessorsAccessGetInsideArray(t *testing.T) { - current := Map{ + m := objx.Map{ "names": []interface{}{ - Map{ + objx.Map{ "first": "Tyler", "last": "Bunnell", }, - Map{ + objx.Map{ "first": "Capitol", "last": "Bollocks", }, }, } - assert.Equal(t, "Tyler", current.Get("names[0].first").Data()) - assert.Equal(t, "Bunnell", current.Get("names[0].last").Data()) - assert.Equal(t, "Capitol", current.Get("names[1].first").Data()) - assert.Equal(t, "Bollocks", current.Get("names[1].last").Data()) + assert.Equal(t, "Tyler", m.Get("names[0].first").Data()) + assert.Equal(t, "Bunnell", m.Get("names[0].last").Data()) + assert.Equal(t, "Capitol", m.Get("names[1].first").Data()) + assert.Equal(t, "Bollocks", m.Get("names[1].last").Data()) - assert.Nil(t, current.Get("names[2]").Data()) + assert.Nil(t, m.Get("names[2]").Data()) } func TestAccessorsGet(t *testing.T) { - current := Map{"name": "Tyler"} + m := objx.Map{"name": "Tyler"} - assert.Equal(t, "Tyler", current.Get("name").Data()) + assert.Equal(t, "Tyler", m.Get("name").Data()) } func TestAccessorsAccessSetSingleField(t *testing.T) { - current := Map{"name": "Tyler"} + m := objx.Map{"name": "Tyler"} - current.Set("name", "Mat") - current.Set("age", 29) + m.Set("name", "Mat") + m.Set("age", 29) - assert.Equal(t, current["name"], "Mat") - assert.Equal(t, current["age"], 29) + assert.Equal(t, m["name"], "Mat") + assert.Equal(t, m["age"], 29) } func TestAccessorsAccessSetSingleFieldNotExisting(t *testing.T) { - current := Map{ + m := objx.Map{ "first": "Tyler", "last": "Bunnell", } - current.Set("name", "Mat") + m.Set("name", "Mat") - assert.Equal(t, current["name"], "Mat") + assert.Equal(t, m["name"], "Mat") } func TestAccessorsAccessSetDeep(t *testing.T) { - current := Map{ - "name": Map{ + m := objx.Map{ + "name": objx.Map{ "first": "Tyler", "last": "Bunnell", }, } - current.Set("name.first", "Mat") - current.Set("name.last", "Ryer") + m.Set("name.first", "Mat") + m.Set("name.last", "Ryer") - assert.Equal(t, "Mat", current.Get("name.first").Data()) - assert.Equal(t, "Ryer", current.Get("name.last").Data()) + assert.Equal(t, "Mat", m.Get("name.first").Data()) + assert.Equal(t, "Ryer", m.Get("name.last").Data()) } func TestAccessorsAccessSetDeepDeep(t *testing.T) { - current := Map{ - "one": Map{ - "two": Map{ - "three": Map{ - "four": 4}, + m := objx.Map{ + "one": objx.Map{ + "two": objx.Map{ + "three": objx.Map{ + "four": 4, + }, }, }, } - current.Set("one.two.three.four", 5) + m.Set("one.two.three.four", 5) - assert.Equal(t, 5, current.Get("one.two.three.four").Data()) + assert.Equal(t, 5, m.Get("one.two.three.four").Data()) } func TestAccessorsAccessSetArray(t *testing.T) { - current := Map{ + m := objx.Map{ "names": []interface{}{"Tyler"}, } - current.Set("names[0]", "Mat") + m.Set("names[0]", "Mat") - assert.Equal(t, "Mat", current.Get("names[0]").Data()) + assert.Equal(t, "Mat", m.Get("names[0]").Data()) } func TestAccessorsAccessSetInsideArray(t *testing.T) { - current := Map{ + m := objx.Map{ "names": []interface{}{ - Map{ + objx.Map{ "first": "Tyler", "last": "Bunnell", }, - Map{ + objx.Map{ "first": "Capitol", "last": "Bollocks", }, }, } - current.Set("names[0].first", "Mat") - current.Set("names[0].last", "Ryer") - current.Set("names[1].first", "Captain") - current.Set("names[1].last", "Underpants") + m.Set("names[0].first", "Mat") + m.Set("names[0].last", "Ryer") + m.Set("names[1].first", "Captain") + m.Set("names[1].last", "Underpants") - assert.Equal(t, "Mat", current.Get("names[0].first").Data()) - assert.Equal(t, "Ryer", current.Get("names[0].last").Data()) - assert.Equal(t, "Captain", current.Get("names[1].first").Data()) - assert.Equal(t, "Underpants", current.Get("names[1].last").Data()) + assert.Equal(t, "Mat", m.Get("names[0].first").Data()) + assert.Equal(t, "Ryer", m.Get("names[0].last").Data()) + assert.Equal(t, "Captain", m.Get("names[1].first").Data()) + assert.Equal(t, "Underpants", m.Get("names[1].last").Data()) } func TestAccessorsSet(t *testing.T) { - current := Map{"name": "Tyler"} + m := objx.Map{"name": "Tyler"} - current.Set("name", "Mat") + m.Set("name", "Mat") - assert.Equal(t, "Mat", current.Get("name").data) + assert.Equal(t, "Mat", m.Get("name").Data()) } From 71bdc84c870156d9df585cc299d5257b83c40bff Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Fri, 26 Jan 2018 10:12:46 +0100 Subject: [PATCH 3/4] Add more tests --- accessors_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/accessors_test.go b/accessors_test.go index 1bd2133..965e4e7 100644 --- a/accessors_test.go +++ b/accessors_test.go @@ -65,6 +65,12 @@ func TestAccessorsAccessGetInsideArray(t *testing.T) { assert.Equal(t, "Bollocks", m.Get("names[1].last").Data()) assert.Nil(t, m.Get("names[2]").Data()) + assert.Nil(t, m.Get("names[]").Data()) + assert.Nil(t, m.Get("names1]]").Data()) + assert.Nil(t, m.Get("names[1]]").Data()) + assert.Nil(t, m.Get("names[[1]]").Data()) + assert.Nil(t, m.Get("names[[1]").Data()) + assert.Nil(t, m.Get("names[[1").Data()) } func TestAccessorsGet(t *testing.T) { @@ -79,8 +85,8 @@ func TestAccessorsAccessSetSingleField(t *testing.T) { m.Set("name", "Mat") m.Set("age", 29) - assert.Equal(t, m["name"], "Mat") - assert.Equal(t, m["age"], 29) + assert.Equal(t, m.Get("name").Data(), "Mat") + assert.Equal(t, m.Get("age").Data(), 29) } func TestAccessorsAccessSetSingleFieldNotExisting(t *testing.T) { @@ -91,7 +97,7 @@ func TestAccessorsAccessSetSingleFieldNotExisting(t *testing.T) { m.Set("name", "Mat") - assert.Equal(t, m["name"], "Mat") + assert.Equal(t, m.Get("name").Data(), "Mat") } func TestAccessorsAccessSetDeep(t *testing.T) { From 50ac195e07eb7e0f26fab1f9553525fb1d68db36 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Fri, 26 Jan 2018 10:18:00 +0100 Subject: [PATCH 4/4] Refactor access methode --- accessors.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/accessors.go b/accessors.go index 41f9031..2864c37 100644 --- a/accessors.go +++ b/accessors.go @@ -46,30 +46,33 @@ func (m Map) Set(selector string, value interface{}) Map { return m } +// getIndex returns the index, which is hold in s by two braches. +// It also returns s withour the index part, e.g. name[1] will return (1, name). +// If no index is found, -1 is returned +func getIndex(s string) (int, string) { + arrayMatches := arrayAccesRegex.FindStringSubmatch(s) + if len(arrayMatches) > 0 { + // Get the key into the map + selector := arrayMatches[1] + // Get the index into the array at the key + // We know this cannt fail because arrayMatches[2] is an int for sure + index, _ := strconv.Atoi(arrayMatches[2]) + return index, selector + } + return -1, s +} + // access accesses the object using the selector and performs the // appropriate action. func access(current interface{}, selector string, value interface{}, isSet bool) interface{} { selSegs := strings.SplitN(selector, PathSeparator, 2) thisSel := selSegs[0] index := -1 - var err error if strings.Contains(thisSel, "[") { - arrayMatches := arrayAccesRegex.FindStringSubmatch(thisSel) - if len(arrayMatches) > 0 { - // Get the key into the map - thisSel = arrayMatches[1] - - // Get the index into the array at the key - index, err = strconv.Atoi(arrayMatches[2]) - - if err != nil { - // This should never happen. If it does, something has gone - // seriously wrong. Panic. - panic("objx: Array index is not an integer. Must use array[int].") - } - } + index, thisSel = getIndex(thisSel) } + if curMap, ok := current.(Map); ok { current = map[string]interface{}(curMap) }