Skip to content

Commit a5f7453

Browse files
Dragomir-IvanovDragomir-Ivanov
committed
Prevent removing of (Required,Default) fields in HTTP PUT. Fixes rs#174 
1 parent 5003f4f commit a5f7453

File tree

3 files changed

+324
-21
lines changed

3 files changed

+324
-21
lines changed

‎README.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ The REST Layer framework is composed of several sub-packages:
5252
- [Authentication & Authorization](#authentication-and-authorization)
5353
- [Conditional Requests](#conditional-requests)
5454
- [Data Integrity & Concurrency Control](#data-integrity-and-concurrency-control)
55+
- [HTTP Methods](#http-methods)
56+
- [HEAD](#head)
57+
- [GET](#get)
58+
- [POST](#post)
59+
- [PUT](#put)
60+
- [PATCH](#patch)
61+
- [DELETE](#delete)
62+
- [OPTIONS](#options)
5563
- [Data Validation](#data-validation)
5664
- [Nullable Values](#nullable-values)
5765
- [Extensible Data Validation](#extensible-data-validation)
@@ -1160,6 +1168,32 @@ This time the update operation was accepted and we got a new `ETag` for the upda
11601168
11611169
Concurrency control header `If-Match` can be used with all mutation methods on item URLs: `PATCH` (update), `PUT` (replace) and `DELETE` (delete).
11621170
1171+
## HTTP Methods
1172+
1173+
Following HTTP Methods are supported currently.
1174+
1175+
### HEAD
1176+
The same as [GET](#get), except that it doesn't return any body.
1177+
1178+
### GET
1179+
Used to query a resource with its sub/embedded resources.
1180+
1181+
### POST
1182+
Used to create new resource document, where new `ID` is generated from the server.
1183+
1184+
### PUT
1185+
Used to create/update single resource document given its `ID`.\
1186+
Be aware when dealing with resource fields with `Default` set. Initial creation for such resources will set particular field to its default value if omitted, however on subsequent `PUT` calls this field will be deleted if omitted. If persistent `Default` field is needed use `{Required: true}` with it.
1187+
1188+
### PATCH
1189+
Used to update/patch single resource document given its `ID`.
1190+
1191+
### DELETE
1192+
Used to delete single resource document given its `ID`, or via [Query](#quering).
1193+
1194+
### OPTIONS
1195+
Used to tell the client, which HTTP Methods are supported on a given resource.
1196+
11631197
## Data Validation
11641198
11651199
Data validation is provided out-of-the-box. Your configuration includes a schema definition for every resource managed by the API. Data sent to the API to be inserted/updated will be validated against the schema, and a resource will only be updated if validation passes. See [Field Definition](#field-definition) section to know more about how to configure your validators.

‎rest/method_item_put_test.go

Lines changed: 287 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,27 @@ import (
1414
"github.com/rs/rest-layer/schema/query"
1515
)
1616

17+
func checkPayload(name string, id interface{}, payload map[string]interface{}) requestCheckerFunc {
18+
return func(t *testing.T, vars *requestTestVars) {
19+
var item *resource.Item
20+
21+
s := vars.Storers[name]
22+
q := query.Query{Predicate: query.Predicate{&query.Equal{Field: "id", Value: id}}, Window: &query.Window{Limit: 1}}
23+
if items, err := s.Find(context.Background(), &q); err != nil {
24+
t.Errorf("s.Find failed: %s", err)
25+
return
26+
} else if len(items.Items) != 1 {
27+
t.Errorf("item with ID %v not found", id)
28+
return
29+
} else {
30+
item = items.Items[0]
31+
}
32+
if !reflect.DeepEqual(payload, item.Payload) {
33+
t.Errorf("Unexpected stored payload for item %v:\nexpect: %#v\ngot: %#v", id, payload, item.Payload)
34+
}
35+
}
36+
}
37+
1738
func TestPutItem(t *testing.T) {
1839
now := time.Now()
1940
yesterday := now.Add(-24 * time.Hour)
@@ -50,26 +71,6 @@ func TestPutItem(t *testing.T) {
5071
Storers: map[string]resource.Storer{"foo": s1, "foo.sub": s2},
5172
}
5273
}
53-
checkPayload := func(name string, id interface{}, payload map[string]interface{}) requestCheckerFunc {
54-
return func(t *testing.T, vars *requestTestVars) {
55-
var item *resource.Item
56-
57-
s := vars.Storers[name]
58-
q := query.Query{Predicate: query.Predicate{&query.Equal{Field: "id", Value: id}}, Window: &query.Window{Limit: 1}}
59-
if items, err := s.Find(context.Background(), &q); err != nil {
60-
t.Errorf("s.Find failed: %s", err)
61-
return
62-
} else if len(items.Items) != 1 {
63-
t.Errorf("item with ID %v not found", id)
64-
return
65-
} else {
66-
item = items.Items[0]
67-
}
68-
if !reflect.DeepEqual(payload, item.Payload) {
69-
t.Errorf("Unexpected stored payload for item %v:\nexpect: %#v\ngot: %#v", id, payload, item.Payload)
70-
}
71-
}
72-
}
7374

7475
tests := map[string]requestTest{
7576
`NoStorage`: {
@@ -301,3 +302,269 @@ func TestPutItem(t *testing.T) {
301302
t.Run(n, tc.Test)
302303
}
303304
}
305+
306+
func TestPutItemDefault(t *testing.T) {
307+
now := time.Now()
308+
309+
sharedInit := func() *requestTestVars {
310+
s1 := mem.NewHandler()
311+
s1.Insert(context.Background(), []*resource.Item{
312+
{ID: "1", ETag: "a", Updated: now, Payload: map[string]interface{}{"id": "1", "foo": "odd"}},
313+
{ID: "2", ETag: "b", Updated: now, Payload: map[string]interface{}{"id": "2", "foo": "odd", "bar": "value"}},
314+
})
315+
idx := resource.NewIndex()
316+
idx.Bind("foo", schema.Schema{
317+
Fields: schema.Fields{
318+
"id": {Sortable: true, Filterable: true},
319+
"foo": {Filterable: true},
320+
"bar": {Filterable: true, Default: "default"},
321+
},
322+
}, s1, resource.DefaultConf)
323+
return &requestTestVars{
324+
Index: idx,
325+
Storers: map[string]resource.Storer{"foo": s1},
326+
}
327+
}
328+
329+
tests := map[string]requestTest{
330+
`pathID:not-found,body:valid,default:missing`: {
331+
Init: sharedInit,
332+
NewRequest: func() (*http.Request, error) {
333+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
334+
return http.NewRequest("PUT", "/foo/66", body)
335+
},
336+
ResponseCode: http.StatusCreated,
337+
ResponseBody: `{"id": "66", "foo": "baz", "bar": "default"}`,
338+
ExtraTest: checkPayload("foo", "66", map[string]interface{}{"id": "66", "foo": "baz", "bar": "default"}),
339+
},
340+
`pathID:not-found,body:valid,default:set`: {
341+
Init: sharedInit,
342+
NewRequest: func() (*http.Request, error) {
343+
body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
344+
return http.NewRequest("PUT", "/foo/66", body)
345+
},
346+
ResponseCode: http.StatusCreated,
347+
ResponseBody: `{"id": "66", "foo": "baz", "bar": "value"}`,
348+
ExtraTest: checkPayload("foo", "66", map[string]interface{}{"id": "66", "foo": "baz", "bar": "value"}),
349+
},
350+
`pathID:found,body:valid,default:missing`: {
351+
Init: sharedInit,
352+
NewRequest: func() (*http.Request, error) {
353+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
354+
return http.NewRequest("PUT", "/foo/1", body)
355+
},
356+
ResponseCode: http.StatusOK,
357+
ResponseBody: `{"id": "1", "foo": "baz"}`,
358+
ExtraTest: checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz"}),
359+
},
360+
`pathID:found,body:valid,default:set`: {
361+
Init: sharedInit,
362+
NewRequest: func() (*http.Request, error) {
363+
body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
364+
return http.NewRequest("PUT", "/foo/1", body)
365+
},
366+
ResponseCode: http.StatusOK,
367+
ResponseBody: `{"id": "1", "foo": "baz", "bar": "value"}`,
368+
ExtraTest: checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz", "bar": "value"}),
369+
},
370+
`pathID:found,body:valid,default:delete`: {
371+
Init: sharedInit,
372+
NewRequest: func() (*http.Request, error) {
373+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
374+
return http.NewRequest("PUT", "/foo/2", body)
375+
},
376+
ResponseCode: http.StatusOK,
377+
ResponseBody: `{"id": "2", "foo": "baz"}`,
378+
ExtraTest: checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz"}),
379+
},
380+
}
381+
382+
for n, tc := range tests {
383+
tc := tc // capture range variable
384+
t.Run(n, tc.Test)
385+
}
386+
}
387+
388+
func TestPutItemRequired(t *testing.T) {
389+
now := time.Now()
390+
391+
sharedInit := func() *requestTestVars {
392+
s1 := mem.NewHandler()
393+
s1.Insert(context.Background(), []*resource.Item{
394+
{ID: "1", ETag: "a", Updated: now, Payload: map[string]interface{}{"id": "1", "foo": "odd"}},
395+
{ID: "2", ETag: "b", Updated: now, Payload: map[string]interface{}{"id": "2", "foo": "odd", "bar": "original"}},
396+
})
397+
idx := resource.NewIndex()
398+
idx.Bind("foo", schema.Schema{
399+
Fields: schema.Fields{
400+
"id": {Sortable: true, Filterable: true},
401+
"foo": {Filterable: true},
402+
"bar": {Filterable: true, Required: true},
403+
},
404+
}, s1, resource.DefaultConf)
405+
return &requestTestVars{
406+
Index: idx,
407+
Storers: map[string]resource.Storer{"foo": s1},
408+
}
409+
}
410+
411+
tests := map[string]requestTest{
412+
`pathID:not-found,body:valid,required:missing`: {
413+
Init: sharedInit,
414+
NewRequest: func() (*http.Request, error) {
415+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
416+
return http.NewRequest("PUT", "/foo/66", body)
417+
},
418+
ResponseCode: http.StatusUnprocessableEntity,
419+
ResponseBody: `{
420+
"code": 422,
421+
"message": "Document contains error(s)",
422+
"issues": {
423+
"bar": ["required"]
424+
}
425+
}`,
426+
},
427+
`pathID:not-found,body:valid,required:set`: {
428+
Init: sharedInit,
429+
NewRequest: func() (*http.Request, error) {
430+
body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
431+
return http.NewRequest("PUT", "/foo/1", body)
432+
},
433+
ResponseCode: http.StatusOK,
434+
ResponseBody: `{"id": "1", "foo": "baz", "bar": "value"}`,
435+
ExtraTest: checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz", "bar": "value"}),
436+
},
437+
`pathID:found,body:valid,required:missing`: {
438+
Init: sharedInit,
439+
NewRequest: func() (*http.Request, error) {
440+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
441+
return http.NewRequest("PUT", "/foo/1", body)
442+
},
443+
ResponseCode: http.StatusUnprocessableEntity,
444+
ResponseBody: `{
445+
"code": 422,
446+
"message": "Document contains error(s)",
447+
"issues": {
448+
"bar": ["required"]
449+
}
450+
}`,
451+
},
452+
`pathID:found,body:valid,required:change`: {
453+
Init: sharedInit,
454+
NewRequest: func() (*http.Request, error) {
455+
body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value1"}`))
456+
return http.NewRequest("PUT", "/foo/2", body)
457+
},
458+
ResponseCode: http.StatusOK,
459+
ResponseBody: `{"id": "2", "foo": "baz", "bar": "value1"}`,
460+
ExtraTest: checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz", "bar": "value1"}),
461+
},
462+
`pathID:found,body:valid,required:delete`: {
463+
Init: sharedInit,
464+
NewRequest: func() (*http.Request, error) {
465+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
466+
return http.NewRequest("PUT", "/foo/2", body)
467+
},
468+
ResponseCode: http.StatusUnprocessableEntity,
469+
ResponseBody: `{
470+
"code": 422,
471+
"message": "Document contains error(s)",
472+
"issues": {
473+
"bar": ["required"]
474+
}
475+
}`,
476+
},
477+
}
478+
479+
for n, tc := range tests {
480+
tc := tc // capture range variable
481+
t.Run(n, tc.Test)
482+
}
483+
}
484+
485+
func TestPutItemRequiredDefault(t *testing.T) {
486+
now := time.Now()
487+
488+
sharedInit := func() *requestTestVars {
489+
s1 := mem.NewHandler()
490+
s1.Insert(context.Background(), []*resource.Item{
491+
{ID: "1", ETag: "a", Updated: now, Payload: map[string]interface{}{"id": "1", "foo": "odd"}},
492+
{ID: "2", ETag: "b", Updated: now, Payload: map[string]interface{}{"id": "2", "foo": "odd", "bar": "original"}},
493+
})
494+
idx := resource.NewIndex()
495+
idx.Bind("foo", schema.Schema{
496+
Fields: schema.Fields{
497+
"id": {Sortable: true, Filterable: true},
498+
"foo": {Filterable: true},
499+
"bar": {Filterable: true, Required: true, Default: "default"},
500+
},
501+
}, s1, resource.DefaultConf)
502+
return &requestTestVars{
503+
Index: idx,
504+
Storers: map[string]resource.Storer{"foo": s1},
505+
}
506+
}
507+
508+
tests := map[string]requestTest{
509+
`pathID:not-found,body:valid,required-default:missing`: {
510+
Init: sharedInit,
511+
NewRequest: func() (*http.Request, error) {
512+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
513+
return http.NewRequest("PUT", "/foo/66", body)
514+
},
515+
ResponseCode: http.StatusCreated,
516+
ResponseBody: `{"id": "66", "foo": "baz", "bar": "default"}`,
517+
ExtraTest: checkPayload("foo", "66", map[string]interface{}{"id": "66", "foo": "baz", "bar": "default"}),
518+
},
519+
`pathID:not-found,body:valid,required-default:set`: {
520+
Init: sharedInit,
521+
NewRequest: func() (*http.Request, error) {
522+
body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
523+
return http.NewRequest("PUT", "/foo/1", body)
524+
},
525+
ResponseCode: http.StatusOK,
526+
ResponseBody: `{"id": "1", "foo": "baz", "bar": "value"}`,
527+
ExtraTest: checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz", "bar": "value"}),
528+
},
529+
`pathID:found,body:valid,required-default:missing`: {
530+
Init: sharedInit,
531+
NewRequest: func() (*http.Request, error) {
532+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
533+
return http.NewRequest("PUT", "/foo/1", body)
534+
},
535+
ResponseCode: http.StatusUnprocessableEntity,
536+
ResponseBody: `{
537+
"code": 422,
538+
"message": "Document contains error(s)",
539+
"issues": {
540+
"bar": ["required"]
541+
}
542+
}`,
543+
},
544+
`pathID:found,body:valid,required-default:change`: {
545+
Init: sharedInit,
546+
NewRequest: func() (*http.Request, error) {
547+
body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
548+
return http.NewRequest("PUT", "/foo/2", body)
549+
},
550+
ResponseCode: http.StatusOK,
551+
ResponseBody: `{"id": "2", "foo": "baz", "bar": "value"}`,
552+
ExtraTest: checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz", "bar": "value"}),
553+
},
554+
`pathID:found,body:valid,required-default:delete`: {
555+
Init: sharedInit,
556+
NewRequest: func() (*http.Request, error) {
557+
body := bytes.NewReader([]byte(`{"foo": "baz"}`))
558+
return http.NewRequest("PUT", "/foo/2", body)
559+
},
560+
ResponseCode: http.StatusOK,
561+
ResponseBody: `{"id": "2", "foo": "baz", "bar": "default"}`,
562+
ExtraTest: checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz", "bar": "default"}),
563+
},
564+
}
565+
566+
for n, tc := range tests {
567+
tc := tc // capture range variable
568+
t.Run(n, tc.Test)
569+
}
570+
}

‎schema/schema.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@ func (s Schema) Prepare(ctx context.Context, payload map[string]interface{}, ori
132132
// previous value as the client would have no way to resubmit the stored value.
133133
if def.Hidden && !def.ReadOnly {
134134
changes[field] = oValue
135+
} else if def.Required && def.Default != nil {
136+
changes[field] = def.Default
135137
} else {
136138
changes[field] = Tombstone
137139
}
@@ -227,7 +229,7 @@ func (s Schema) validate(changes map[string]interface{}, base map[string]interfa
227229
}
228230
// Check required fields.
229231
if def.Required {
230-
if value, found := changes[field]; !found || value == nil {
232+
if value, found := changes[field]; !found || value == nil || value == Tombstone {
231233
if found {
232234
// If explicitly set to null, raise the required error.
233235
addFieldError(errs, field, "required")

0 commit comments

Comments
 (0)