Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite Accessor to fix array access issues, also add a Delete method #142

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
293 changes: 171 additions & 122 deletions accessors.go
Original file line number Diff line number Diff line change
@@ -1,33 +1,21 @@
package objx

import (
"fmt"
"reflect"
"regexp"
"strconv"
"strings"
)

const (
// PathSeparator is the character used to separate the elements
// of the keypath.
//
// For example, `location.address.city`
PathSeparator string = "."
// Regex to parse an array index access
var parseIndexRegex = regexp.MustCompile(`\[([\d]+)\]`)

// arrayAccessRegexString is the regex used to extract the array number
// from the access path
arrayAccessRegexString = `^(.+)\[([0-9]+)\]$`
type notFoundError struct{}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we just make this

var NotFoundError = errors.New("not found")


// mapAccessRegexString is the regex used to extract the map key
// from the access path
mapAccessRegexString = `^([^\[]*)\[([^\]]+)\](.*)$`
)

// arrayAccessRegex is the compiled arrayAccessRegexString
var arrayAccessRegex = regexp.MustCompile(arrayAccessRegexString)

// mapAccessRegex is the compiled mapAccessRegexString
var mapAccessRegex = regexp.MustCompile(mapAccessRegexString)
func (m *notFoundError) Error() string {
return "NotFound"
}

// Get gets the value using the specified selector and
// returns it inside a new Obj object.
Expand All @@ -43,7 +31,7 @@ var mapAccessRegex = regexp.MustCompile(mapAccessRegexString)
//
// o.Get("books[1].chapters[2].title")
func (m Map) Get(selector string) *Value {
rawObj := access(m, selector, nil, false)
rawObj, _ := access(m, selector, nil, false)
return &Value{data: rawObj}
}

Expand All @@ -58,140 +46,201 @@ func (m Map) Get(selector string) *Value {
//
// o.Set("books[1].chapters[2].title","Time to Go")
func (m Map) Set(selector string, value interface{}) Map {
access(m, selector, value, true)
var newObj reflect.Value
if value == nil {
newObj = reflect.ValueOf(&value).Elem()
} else {
newObj = reflect.ValueOf(value)
}
access(m, selector, &newObj, true)
return m
}

// getIndex returns the index, which is hold in s by two branches.
// It also returns s without 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 := arrayAccessRegex.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 can't fail because arrayMatches[2] is an int for sure
index, _ := strconv.Atoi(arrayMatches[2])
return index, selector
// Has gets whether there is something at the specified selector
// or not.
//
// If m is nil, Has will always return false.
func (m Map) Has(selector string) bool {
if m == nil {
return false
}
return -1, s
_, err := access(m, selector, nil, false)
return err == nil
}

// getKey returns the key which is held in s by two brackets.
// It also returns the next selector.
func getKey(s string) (string, string) {
selSegs := strings.SplitN(s, PathSeparator, 2)
thisSel := selSegs[0]
nextSel := ""
// Deletes the value from the element
// Note: Array elements can not be deleted, they will only be set null
// Returns the old element or nil if it did not exist
func (m Map) Delete(selector string) *Value {
val := reflect.ValueOf(nil)
res, _ := access(m, selector, &val, false)
return &Value{data: res}
}

if len(selSegs) > 1 {
nextSel = selSegs[1]
}
func parsePath(path string) ([]string, error) {
res := make([]string, 0, 8)
path = strings.TrimPrefix(path, ".")

mapMatches := mapAccessRegex.FindStringSubmatch(s)
if len(mapMatches) > 0 {
if _, err := strconv.Atoi(mapMatches[2]); err != nil {
thisSel = mapMatches[1]
nextSel = "[" + mapMatches[2] + "]" + mapMatches[3]
for {
pos := strings.IndexAny(path, ".[")

if thisSel == "" {
thisSel = mapMatches[2]
nextSel = mapMatches[3]
if pos == 0 && path[pos] == '[' {
pos = strings.IndexAny(path[1:], "[]")
if pos < 0 || path[pos+1] == '[' {
return nil, fmt.Errorf("invalid path")
}
pos += 2
}
var elem string
if pos < 0 {
elem = path
} else {
elem = path[:pos]
}

if nextSel == "" {
selSegs = []string{"", ""}
} else if nextSel[0] == '.' {
nextSel = nextSel[1:]
if elem[0] == '[' {
if !parseIndexRegex.MatchString(elem) {
// its not an index so drop the backets for normal access
if len(elem) <= 2 {
return nil, fmt.Errorf("invalid path")
}
elem = elem[1 : len(elem)-1]
}
}
}

return thisSel, nextSel
}
res = append(res, elem)

// access accesses the object using the selector and performs the
// appropriate action.
func access(current interface{}, selector string, value interface{}, isSet bool) interface{} {
thisSel, nextSel := getKey(selector)

indexes := []int{}
for strings.Contains(thisSel, "[") {
prevSel := thisSel
index := -1
index, thisSel = getIndex(thisSel)
indexes = append(indexes, index)
if prevSel == thisSel {
if pos < 0 || pos >= len(path) {
break
}
}

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 nextSel == "" && isSet {
curMSI[thisSel] = value
return nil
if path[pos] == '.' {
pos++
}
path = path[pos:]
}
return res, nil
}

_, ok := curMSI[thisSel].(map[string]interface{})
if !ok {
_, ok = curMSI[thisSel].(Map)
func getArrayIndex(key string) int {
if key[0] == '[' {
idx, err := strconv.ParseUint(key[1:len(key)-1], 10, 64)
if err != nil {
// should not happen, otherwise the path is invalid
panic(err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still rather not panic and instead return error here.

}
return int(idx)
}
return -1
}

if (curMSI[thisSel] == nil || !ok) && len(indexes) == 0 && isSet {
curMSI[thisSel] = map[string]interface{}{}
}
func max(a, b int) int {
if a > b {
return a
}
return b
}

current = curMSI[thisSel]
default:
current = nil
func access(object interface{}, selector string, value *reflect.Value, createPath bool) (interface{}, error) {
path, err := parsePath(selector)
if err != nil {
return nil, err
}

// do we need to access the item of an array?
if len(indexes) > 0 {
num := len(indexes)
for num > 0 {
num--
index := indexes[num]
indexes = indexes[:num]
if array, ok := interSlice(current); ok {
if index < len(array) {
current = array[index]
length := len(path)
lastIndex := length - 1

currentObj := reflect.ValueOf(object)
for index := 0; index < length && currentObj.IsValid(); index++ {
key := path[index]
arrayIndex := getArrayIndex(key)
var nextObj reflect.Value
// fmt.Printf("current: %s %v\n", key, currentObj.Kind())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// fmt.Printf("current: %s %v\n", key, currentObj.Kind())

if arrayIndex >= 0 {
if currentObj.Kind() != reflect.Slice || currentObj.Len() <= arrayIndex {
if createPath {
return nil, fmt.Errorf("set with invalid type. Expected currentObj to be a Slice(Len > %d) but got %v(Len %d)", arrayIndex, currentObj.Kind(), currentObj.Len())
}
return nil, &notFoundError{}
}
nextObj = currentObj.Index(arrayIndex)
} else {
if currentObj.Kind() != reflect.Map {
if createPath {
return nil, fmt.Errorf("set with invalid type. Expected currentObj to be a Map but got %v", currentObj.Kind())
}
return nil, &notFoundError{}
}
nextObj = currentObj.MapIndex(reflect.ValueOf(key))
if !createPath && !nextObj.IsValid() {
return nil, &notFoundError{}
}
}
if nextObj.Kind() == reflect.Interface {
nextObj = nextObj.Elem()
}
// fmt.Printf("key: %s %v\n", key, nextObj.Kind())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// fmt.Printf("key: %s %v\n", key, nextObj.Kind())


newObj := nextObj
if index == lastIndex && value != nil {
// we are in the last path, assign the value
newObj = *value
} else {
if createPath {
nextArrayIndex := getArrayIndex(path[index+1])
if nextArrayIndex >= 0 {
// next element will be an array
if !nextObj.IsValid() || nextObj.Kind() != reflect.Slice {
// fmt.Printf("new slice for %s\n", key)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// fmt.Printf("new slice for %s\n", key)

newObj = reflect.ValueOf(make([]interface{}, nextArrayIndex+1, max(nextArrayIndex+1, 8)))
} else if nextObj.Len() <= nextArrayIndex {
// fmt.Printf("nextObj %s len %d cap %d\n", key, nextObj.Len(), nextObj.Cap())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// fmt.Printf("nextObj %s len %d cap %d\n", key, nextObj.Len(), nextObj.Cap())

newObj = reflect.AppendSlice(nextObj, reflect.ValueOf(make([]interface{}, nextArrayIndex-nextObj.Len()+1)))
}
} else {
current = nil
break
// next element will be an normal object
if !nextObj.IsValid() || nextObj.Kind() != reflect.Map {
// fmt.Printf("new map for %s\n", key)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// fmt.Printf("new map for %s\n", key)

newObj = reflect.ValueOf(map[string]interface{}{})
}
}
}
}
}

if nextSel != "" {
current = access(current, nextSel, value, isSet)
}
return current
}
if newObj != nextObj || (value != nil && index == lastIndex) {
// fmt.Printf("assign key %s to %v\n", key, newObj)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// fmt.Printf("assign key %s to %v\n", key, newObj)

if arrayIndex >= 0 {
if newObj.IsValid() {
currentObj.Index(arrayIndex).Set(newObj)
} else {
// delete op
// TODO: implement array shrinking if its the last element
//if currentObj.Len() == arrayIndex+1 {
// val := currentObj.Slice(0, arrayIndex)
// currentObj.Set(val)
var val interface{}
currentObj.Index(arrayIndex).Set(reflect.ValueOf(&val).Elem())
currentObj = nextObj
break
}
nextObj = newObj
} else {
currentObj.SetMapIndex(reflect.ValueOf(key), newObj)
if !newObj.IsValid() {
// delete op
currentObj = nextObj
break
}
nextObj = newObj
}

func interSlice(slice interface{}) ([]interface{}, bool) {
if array, ok := slice.([]interface{}); ok {
return array, ok
}
}

s := reflect.ValueOf(slice)
if s.Kind() != reflect.Slice {
return nil, false
currentObj = nextObj
}

ret := make([]interface{}, s.Len())

for i := 0; i < s.Len(); i++ {
ret[i] = s.Index(i).Interface()
if !currentObj.IsValid() {
// JSON NULL
return nil, nil
}

return ret, true
return currentObj.Interface(), nil
}