Skip to content

Commit

Permalink
Fix issue arising from structs 2 or more levels deep.
Browse files Browse the repository at this point in the history 
- Fix fields array and map dot notations using bad paths.
- Add test for 2-level embeds.
- Bump readme version.
  • Loading branch information
@Nifty255
Nifty255 committed May 10, 2020
1 parent 9d998b4 commit 35d5e29
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 30 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
@@ -1,5 +1,5 @@
# Go PATCH
![Project status](https://img.shields.io/badge/version-1.0.0-green.svg)
![Project status](https://img.shields.io/badge/version-1.0.2-green.svg)
[![Build Status](https://travis-ci.org/Nifty255/gopatch.svg?branch=master)](https://travis-ci.org/Nifty255/gopatch)
[![Coverage Status](https://coveralls.io/repos/github/Nifty255/gopatch/badge.svg?branch=master)](https://coveralls.io/github/Nifty255/gopatch?branch=master)
[![GoDoc](https://godoc.org/github.com/Nifty255/gopatch?status.svg)](https://godoc.org/github.com/Nifty255/gopatch)
Expand Down
57 changes: 29 additions & 28 deletions patcher.go
View file
Expand Up @@ -30,10 +30,10 @@ func (p Patcher) Patch(dest interface{}, patch map[string]interface{}) (*PatchRe

return nil, errDestInvalid
}
return p.patch(dest, patch, p.config.PermittedFields, p.config.EmbedPath)
return p.patch(dest, patch, p.config.PermittedFields, true)
}

func (p Patcher) patch(dest interface{}, patch map[string]interface{}, permitted []string, path string) (*PatchResult, error) {
func (p Patcher) patch(dest interface{}, patch map[string]interface{}, permitted []string, root bool) (*PatchResult, error) {

// Get the actual struct data from the pointer and its type data.
valueOfDest := reflect.ValueOf(dest).Elem()
Expand Down Expand Up @@ -72,13 +72,10 @@ func (p Patcher) patch(dest interface{}, patch map[string]interface{}, permitted
// Get the patch value based on the fieldName.
if val, ok := patch[fieldName]; ok {

full := path
if full != "" { full += "."+fieldName } else { full = fieldName }

// Check that the field isn't unpermitted by tag. Doing this before checking the permitted list placed priority on the tag.
if fieldT.Tag.Get("gopatch") == "-" {
if p.config.UnpermittedErrors { return nil, errFieldUnpermitted(fieldName, "gopatch tag") }
results.Unpermitted = append(results.Unpermitted, full)
results.Unpermitted = append(results.Unpermitted, fieldName)
continue
}

Expand All @@ -104,7 +101,7 @@ func (p Patcher) patch(dest interface{}, patch map[string]interface{}, permitted
// Skip field or error if it wasn't permitted.
if !allowed {
if p.config.UnpermittedErrors { return nil, errFieldUnpermitted(fieldName, "permitted array") }
results.Unpermitted = append(results.Unpermitted, full)
results.Unpermitted = append(results.Unpermitted, fieldName)
continue
}
}
Expand All @@ -116,7 +113,7 @@ func (p Patcher) patch(dest interface{}, patch map[string]interface{}, permitted
fieldV.Set(v)

// Add data about the successful update to the results.
if err := p.saveToResults(&results, fieldT, val, path); err != nil { return nil, err }
if err := p.saveToResults(&results, fieldT, val, root); err != nil { return nil, err }

// Next!
continue
Expand All @@ -132,7 +129,7 @@ func (p Patcher) patch(dest interface{}, patch map[string]interface{}, permitted
if updateSuccess {

// Add data about the successful update to the results.
if err := p.saveToResults(&results, fieldT, val, path); err != nil { return nil, err }
if err := p.saveToResults(&results, fieldT, val, root); err != nil { return nil, err }

// Next!
continue
Expand Down Expand Up @@ -163,21 +160,21 @@ func (p Patcher) patch(dest interface{}, patch map[string]interface{}, permitted

// Patch the field, even if it was reset, by recursion.
if !fieldV.CanAddr() { continue }
deep, err := p.patch(fieldV.Addr().Interface(), val.(map[string]interface{}), getPermittedAtPath(permitted, full), path)
deep, err := p.patch(fieldV.Addr().Interface(), val.(map[string]interface{}), getPermittedInEmbedded(permitted, fieldName), false)

// If an error occurred while deep-patching, bubble up immediately.
if err != nil { return nil, err }

// Merge deep-patched results into the current results.
if err := p.mergeResults(&results, deep, fieldT, replace, path); err != nil { return nil, err }
if err := p.mergeResults(&results, deep, fieldT, replace, root); err != nil { return nil, err }
}
}
}

return &results, nil
}

func (p *Patcher) saveToResults(r *PatchResult, dest reflect.StructField, patch interface{}, path string) error {
func (p *Patcher) saveToResults(r *PatchResult, dest reflect.StructField, patch interface{}, root bool) error {

// Get a field name for the fields array.
fieldName := dest.Name
Expand All @@ -191,6 +188,9 @@ func (p *Patcher) saveToResults(r *PatchResult, dest reflect.StructField, patch
}
}

// Prepend the embed path if this is root and there is an embed path.
if root && p.config.EmbedPath != "" { fieldName = p.config.EmbedPath+"."+fieldName }

// Append.
r.Fields = append(r.Fields, fieldName)

Expand All @@ -206,13 +206,16 @@ func (p *Patcher) saveToResults(r *PatchResult, dest reflect.StructField, patch
}
}

// Add to map, prepended with a path if specified, otherwise with only the field name.
if path == "" { r.Map[fieldName] = patch } else { r.Map[path+"."+fieldName] = patch }
// Prepend the embed path if this is root and there is an embed path.
if root && p.config.EmbedPath != "" { fieldName = p.config.EmbedPath+"."+fieldName }

// Add to map.
r.Map[fieldName] = patch

return nil
}

func (p *Patcher) mergeResults(top, deep *PatchResult, dest reflect.StructField, replace bool, path string) error {
func (p *Patcher) mergeResults(top, deep *PatchResult, dest reflect.StructField, replace, root bool) error {

// Get a field name for the fields array.
fieldName := dest.Name
Expand All @@ -226,15 +229,15 @@ func (p *Patcher) mergeResults(top, deep *PatchResult, dest reflect.StructField,
}
}

full := path
if full != "" { full += "."+fieldName } else { full = fieldName }
// Prepend the embed path if this is root and there is an embed path.
if root && p.config.EmbedPath != "" { fieldName = p.config.EmbedPath+"."+fieldName }

// Map field names to path and append.
if replace {
top.Fields = append(top.Fields, path+"."+fieldName)
top.Fields = append(top.Fields, fieldName)
} else {
for _, field := range(deep.Fields) {
top.Fields = append(top.Fields, full+"."+field)
top.Fields = append(top.Fields, fieldName+"."+field)
}
}

Expand All @@ -250,33 +253,31 @@ func (p *Patcher) mergeResults(top, deep *PatchResult, dest reflect.StructField,
}
}

full = path
if full != "" { full += "."+fieldName } else { full = fieldName }
// Prepend the embed path if this is root.
if root && p.config.EmbedPath != "" { fieldName = p.config.EmbedPath+"."+fieldName }

if replace {
top.Map[full] = deep.Map
top.Map[fieldName] = deep.Map
} else {
for k, v := range(deep.Map) {
top.Map[full+"."+k] = v
top.Map[fieldName+"."+k] = v
}
}

return nil
}

func getPermittedAtPath(permitted []string, path string) []string {
func getPermittedInEmbedded(permitted []string, fieldName string) []string {

if len(permitted) == 0 { return []string{ "*" } }

if path == "" { return permitted }

out := make([]string, 0, len(permitted))

for _, permitted := range(permitted) {

if permitted == path+".*" { return []string{ "*" } }
if permitted == fieldName+".*" { return []string{ "*" } }

if strings.HasPrefix(permitted, path+".") { out = append(out, permitted[len(path)+1:])}
if strings.HasPrefix(permitted, fieldName+".") { out = append(out, permitted[len(fieldName)+1:])}
}

return out
Expand Down
49 changes: 48 additions & 1 deletion patcher_test.go
View file
Expand Up @@ -8,10 +8,16 @@ import(

func TestPatcher(t *testing.T) {

type TestDouble struct {
Field1 string
Field2 int
}

type TestEmbedded struct {
Field1 string
Field2 int
Field3 bool
Field4 TestDouble
}

type TestStruct struct {
Expand Down Expand Up @@ -400,7 +406,7 @@ func TestPatcher(t *testing.T) {

// Test to see if the resulting update map is correct, and only contains the permitted field.
if v, e := result.Map["Field5.Field2"]; !e || v != 255 || len(result.Map) > 1 {
t.Errorf("Expected patch result map to contain exactly \"Field5.Field2\": \"test\". Contained %v", result.Map)
t.Errorf("Expected patch result map to contain exactly \"Field5.Field2\": 255. Contained %v", result.Map)
return
}
})
Expand Down Expand Up @@ -443,4 +449,45 @@ func TestPatcher(t *testing.T) {
return
}
})

t.Run("embedded-double", func(t *testing.T) {

cfg := PatcherConfig{}

patcher := New(cfg)

testInstance := TestStruct{
Field4: TestEmbedded{
Field4: TestDouble{
Field1: "test",
},
},
}

result, err := patcher.Patch(&testInstance, map[string]interface{}{
"Field4": map[string]interface{}{
"Field4": map[string]interface{}{
"Field2": 255,
},
},
})

// Test for unexpected errors.
if err != nil {
t.Errorf("Unexpected patch error: %q", err.Error())
return
}

// Test to see if the instance was patched.
if testInstance.Field4.Field4.Field1 != "test" || testInstance.Field4.Field4.Field2 != 255 {
t.Errorf("Expected patch to only patch Field4.Field2. Patch affected struct so: %v", testInstance)
return
}

// Test to see if the resulting update map is correct, and only contains the permitted field.
if v, e := result.Map["Field4.Field4.Field2"]; !e || v != 255 || len(result.Map) > 1 {
t.Errorf("Expected patch result map to contain exactly \"Field4.Field4.Field2\": 255. Contained %v", result.Map)
return
}
})
}

0 comments on commit 35d5e29

Please sign in to comment.