Prevent removing of (Required,Default) fields in HTTP PUT. Fixes rs#174

README.md

@@ -52,6 +52,14 @@ The REST Layer framework is composed of several sub-packages:
 - [Authentication & Authorization](#authentication-and-authorization)
 - [Conditional Requests](#conditional-requests)
 - [Data Integrity & Concurrency Control](#data-integrity-and-concurrency-control)
+- [HTTP Methods](#http-methods)
+  - [HEAD](#head)
+  - [GET](#get)
+  - [POST](#post)
+  - [PUT](#put)
+  - [PATCH](#patch)
+  - [DELETE](#delete)
+  - [OPTIONS](#options)
 - [Data Validation](#data-validation)
   - [Nullable Values](#nullable-values)
   - [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
 
 Concurrency control header `If-Match` can be used with all mutation methods on item URLs: `PATCH` (update), `PUT` (replace) and `DELETE` (delete).
 
+## HTTP Methods
+
+Following HTTP Methods are supported currently.
+
+### HEAD
+The same as [GET](#get), except that it doesn't return any body.
+
+### GET
+Used to query a resource with its sub/embedded resources.
+
+### POST
+Used to create new resource document, where new `ID` is generated from the server.
+
+### PUT
+Used to create/update single resource document given its `ID`.\
+Be aware when dealing with resource fields marked with `{Default: true}`. 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, Default: true}`. 
+
+### PATCH
+Used to update/patch single resource document given its `ID`.
+
+### DELETE
+Used to delete single resource document given its `ID`, or via [Query](#quering).
+
+### OPTIONS
+Used to tell the client, which of available HTTP Methods are supported on a given resource.
+
 ## Data Validation
 
 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

@@ -14,6 +14,27 @@ import (
 	"github.com/rs/rest-layer/schema/query"
 )
 
+func checkPayload(name string, id interface{}, payload map[string]interface{}) requestCheckerFunc {
+	return func(t *testing.T, vars *requestTestVars) {
+		var item *resource.Item
+
+		s := vars.Storers[name]
+		q := query.Query{Predicate: query.Predicate{&query.Equal{Field: "id", Value: id}}, Window: &query.Window{Limit: 1}}
+		if items, err := s.Find(context.Background(), &q); err != nil {
+			t.Errorf("s.Find failed: %s", err)
+			return
+		} else if len(items.Items) != 1 {
+			t.Errorf("item with ID %v not found", id)
+			return
+		} else {
+			item = items.Items[0]
+		}
+		if !reflect.DeepEqual(payload, item.Payload) {
+			t.Errorf("Unexpected stored payload for item %v:\nexpect: %#v\ngot: %#v", id, payload, item.Payload)
+		}
+	}
+}
+
 func TestPutItem(t *testing.T) {
 	now := time.Now()
 	yesterday := now.Add(-24 * time.Hour)
@@ -50,26 +71,6 @@ func TestPutItem(t *testing.T) {
 			Storers: map[string]resource.Storer{"foo": s1, "foo.sub": s2},
 		}
 	}
-	checkPayload := func(name string, id interface{}, payload map[string]interface{}) requestCheckerFunc {
-		return func(t *testing.T, vars *requestTestVars) {
-			var item *resource.Item
-
-			s := vars.Storers[name]
-			q := query.Query{Predicate: query.Predicate{&query.Equal{Field: "id", Value: id}}, Window: &query.Window{Limit: 1}}
-			if items, err := s.Find(context.Background(), &q); err != nil {
-				t.Errorf("s.Find failed: %s", err)
-				return
-			} else if len(items.Items) != 1 {
-				t.Errorf("item with ID %v not found", id)
-				return
-			} else {
-				item = items.Items[0]
-			}
-			if !reflect.DeepEqual(payload, item.Payload) {
-				t.Errorf("Unexpected stored payload for item %v:\nexpect: %#v\ngot: %#v", id, payload, item.Payload)
-			}
-		}
-	}
 
 	tests := map[string]requestTest{
 		`NoStorage`: {
@@ -301,3 +302,269 @@ func TestPutItem(t *testing.T) {
 		t.Run(n, tc.Test)
 	}
 }
+
+func TestPutItemDefault(t *testing.T) {
+	now := time.Now()
+
+	sharedInit := func() *requestTestVars {
+		s1 := mem.NewHandler()
+		s1.Insert(context.Background(), []*resource.Item{
+			{ID: "1", ETag: "a", Updated: now, Payload: map[string]interface{}{"id": "1", "foo": "odd"}},
+			{ID: "2", ETag: "b", Updated: now, Payload: map[string]interface{}{"id": "2", "foo": "odd", "bar": "value"}},
+		})
+		idx := resource.NewIndex()
+		idx.Bind("foo", schema.Schema{
+			Fields: schema.Fields{
+				"id":  {Sortable: true, Filterable: true},
+				"foo": {Filterable: true},
+				"bar": {Filterable: true, Default: "default"},
+			},
+		}, s1, resource.DefaultConf)
+		return &requestTestVars{
+			Index:   idx,
+			Storers: map[string]resource.Storer{"foo": s1},
+		}
+	}
+
+	tests := map[string]requestTest{
+		`pathID:not-found,body:valid,default:missing`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/66", body)
+			},
+			ResponseCode: http.StatusCreated,
+			ResponseBody: `{"id": "66", "foo": "baz", "bar": "default"}`,
+			ExtraTest:    checkPayload("foo", "66", map[string]interface{}{"id": "66", "foo": "baz", "bar": "default"}),
+		},
+		`pathID:not-found,body:valid,default:set`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
+				return http.NewRequest("PUT", "/foo/66", body)
+			},
+			ResponseCode: http.StatusCreated,
+			ResponseBody: `{"id": "66", "foo": "baz", "bar": "value"}`,
+			ExtraTest:    checkPayload("foo", "66", map[string]interface{}{"id": "66", "foo": "baz", "bar": "value"}),
+		},
+		`pathID:found,body:valid,default:missing`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/1", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "1", "foo": "baz"}`,
+			ExtraTest:    checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz"}),
+		},
+		`pathID:found,body:valid,default:set`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
+				return http.NewRequest("PUT", "/foo/1", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "1", "foo": "baz", "bar": "value"}`,
+			ExtraTest:    checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz", "bar": "value"}),
+		},
+		`pathID:found,body:valid,default:delete`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/2", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "2", "foo": "baz"}`,
+			ExtraTest:    checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz"}),
+		},
+	}
+
+	for n, tc := range tests {
+		tc := tc // capture range variable
+		t.Run(n, tc.Test)
+	}
+}
+
+func TestPutItemRequired(t *testing.T) {
+	now := time.Now()
+
+	sharedInit := func() *requestTestVars {
+		s1 := mem.NewHandler()
+		s1.Insert(context.Background(), []*resource.Item{
+			{ID: "1", ETag: "a", Updated: now, Payload: map[string]interface{}{"id": "1", "foo": "odd"}},
+			{ID: "2", ETag: "b", Updated: now, Payload: map[string]interface{}{"id": "2", "foo": "odd", "bar": "original"}},
+		})
+		idx := resource.NewIndex()
+		idx.Bind("foo", schema.Schema{
+			Fields: schema.Fields{
+				"id":  {Sortable: true, Filterable: true},
+				"foo": {Filterable: true},
+				"bar": {Filterable: true, Required: true},
+			},
+		}, s1, resource.DefaultConf)
+		return &requestTestVars{
+			Index:   idx,
+			Storers: map[string]resource.Storer{"foo": s1},
+		}
+	}
+
+	tests := map[string]requestTest{
+		`pathID:not-found,body:valid,required:missing`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/66", body)
+			},
+			ResponseCode: http.StatusUnprocessableEntity,
+			ResponseBody: `{
+				"code": 422,
+				"message": "Document contains error(s)",
+				"issues": {
+					"bar": ["required"]
+				}
+			}`,
+		},
+		`pathID:not-found,body:valid,required:set`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
+				return http.NewRequest("PUT", "/foo/1", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "1", "foo": "baz", "bar": "value"}`,
+			ExtraTest:    checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz", "bar": "value"}),
+		},
+		`pathID:found,body:valid,required:missing`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/1", body)
+			},
+			ResponseCode: http.StatusUnprocessableEntity,
+			ResponseBody: `{
+				"code": 422,
+				"message": "Document contains error(s)",
+				"issues": {
+					"bar": ["required"]
+				}
+			}`,
+		},
+		`pathID:found,body:valid,required:change`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value1"}`))
+				return http.NewRequest("PUT", "/foo/2", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "2", "foo": "baz", "bar": "value1"}`,
+			ExtraTest:    checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz", "bar": "value1"}),
+		},
+		`pathID:found,body:valid,required:delete`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/2", body)
+			},
+			ResponseCode: http.StatusUnprocessableEntity,
+			ResponseBody: `{
+				"code": 422,
+				"message": "Document contains error(s)",
+				"issues": {
+					"bar": ["required"]
+				}
+			}`,
+		},
+	}
+
+	for n, tc := range tests {
+		tc := tc // capture range variable
+		t.Run(n, tc.Test)
+	}
+}
+
+func TestPutItemRequiredDefault(t *testing.T) {
+	now := time.Now()
+
+	sharedInit := func() *requestTestVars {
+		s1 := mem.NewHandler()
+		s1.Insert(context.Background(), []*resource.Item{
+			{ID: "1", ETag: "a", Updated: now, Payload: map[string]interface{}{"id": "1", "foo": "odd"}},
+			{ID: "2", ETag: "b", Updated: now, Payload: map[string]interface{}{"id": "2", "foo": "odd", "bar": "original"}},
+		})
+		idx := resource.NewIndex()
+		idx.Bind("foo", schema.Schema{
+			Fields: schema.Fields{
+				"id":  {Sortable: true, Filterable: true},
+				"foo": {Filterable: true},
+				"bar": {Filterable: true, Required: true, Default: "default"},
+			},
+		}, s1, resource.DefaultConf)
+		return &requestTestVars{
+			Index:   idx,
+			Storers: map[string]resource.Storer{"foo": s1},
+		}
+	}
+
+	tests := map[string]requestTest{
+		`pathID:not-found,body:valid,required-default:missing`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/66", body)
+			},
+			ResponseCode: http.StatusCreated,
+			ResponseBody: `{"id": "66", "foo": "baz", "bar": "default"}`,
+			ExtraTest:    checkPayload("foo", "66", map[string]interface{}{"id": "66", "foo": "baz", "bar": "default"}),
+		},
+		`pathID:not-found,body:valid,required-default:set`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
+				return http.NewRequest("PUT", "/foo/1", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "1", "foo": "baz", "bar": "value"}`,
+			ExtraTest:    checkPayload("foo", "1", map[string]interface{}{"id": "1", "foo": "baz", "bar": "value"}),
+		},
+		`pathID:found,body:valid,required-default:missing`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/1", body)
+			},
+			ResponseCode: http.StatusUnprocessableEntity,
+			ResponseBody: `{
+				"code": 422,
+				"message": "Document contains error(s)",
+				"issues": {
+					"bar": ["required"]
+				}
+			}`,
+		},
+		`pathID:found,body:valid,required-default:change`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz", "bar": "value"}`))
+				return http.NewRequest("PUT", "/foo/2", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "2", "foo": "baz", "bar": "value"}`,
+			ExtraTest:    checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz", "bar": "value"}),
+		},
+		`pathID:found,body:valid,required-default:delete`: {
+			Init: sharedInit,
+			NewRequest: func() (*http.Request, error) {
+				body := bytes.NewReader([]byte(`{"foo": "baz"}`))
+				return http.NewRequest("PUT", "/foo/2", body)
+			},
+			ResponseCode: http.StatusOK,
+			ResponseBody: `{"id": "2", "foo": "baz", "bar": "default"}`,
+			ExtraTest:    checkPayload("foo", "2", map[string]interface{}{"id": "2", "foo": "baz", "bar": "default"}),
+		},
+	}
+
+	for n, tc := range tests {
+		tc := tc // capture range variable
+		t.Run(n, tc.Test)
+	}
+}

schema/schema.go

@@ -132,6 +132,8 @@ func (s Schema) Prepare(ctx context.Context, payload map[string]interface{}, ori
 				// previous value as the client would have no way to resubmit the stored value.
 				if def.Hidden && !def.ReadOnly {
 					changes[field] = oValue
+				} else if def.Required && def.Default != nil {
+					changes[field] = def.Default
 				} else {
 					changes[field] = Tombstone
 				}
@@ -227,7 +229,7 @@ func (s Schema) validate(changes map[string]interface{}, base map[string]interfa
 		}
 		// Check required fields.
 		if def.Required {
-			if value, found := changes[field]; !found || value == nil {
+			if value, found := changes[field]; !found || value == nil || value == Tombstone {
 				if found {
 					// If explicitly set to null, raise the required error.
 					addFieldError(errs, field, "required")