OnInsert/OnUpdate hooks can modify the new resource item, Etag not recalculated

[Dragomir-Ivanov]

During POST/PUT/PATCH new resource item Etag generation is done in item, err := resource.NewItem(doc) from the document supplied from the request. However after that hooks will be applied, that may or may not modify this item further. Modifying the item in the hook, doesn't recalculate the Etag.
One obvious solution suggested by @smyrman , is moving Etag calculation right before hitting the Store.

[smyrman]

Let's try to find a few problem cases, to see how they are handled by any given solution:

E.g. what happens if:

• A resource with an updateAt timestamp field is PATCHED with no changes. Is the resource (and hence the E-tag) updated?

[Dragomir-Ivanov]

Okay, if you are referring to the invisible _updated field, that is produced by rest-layer then no, it is not included in the Etag sum.

func NewItem(payload map[string]interface{}) (*Item, error) {
	id, found := payload["id"]
	if !found {
		return nil, errors.New("Missing ID field")
	}
	etag, err := genEtag(payload)
	if err != nil {
		return nil, err
	}
	item := &Item{
		ID:      id,
		ETag:    etag,
		Updated: time.Now(),
		Payload: payload,
	}
	return item, nil
}

However if you are including your own:
"updated": schema.UpdatedField,, I am afraid it is a modification.

[smyrman]

I am referring to when you include your own field; or indeed any other field that is generated (by a field or resource hook) on update.

Another important question is:

• Is there any cases where it's important for a hook to get an updated/accurate E-tag? (can't think of any myself)

The reason for coming up with these cases is to see which one we want to priortize before settling on the exact solution; there are trade-offs we can do to prioritize the most important use-cases if we can figure out which ones that is.

[smyrman]

Example trade-off:

However if you are including your own: "updated": schema.UpdatedField,, I am afraid it is a modification.

We can avoid this by aborting the update if there are no changes to a resource after a patch or put operation.

Trade-off: It won't be possible to force resource updates (via hooks) by updating (without changing) a field value.

What ever we end up on, the solution must of course be documented. Writing the documentation will also be a good test for the solution; if we can't easily formulate the behaviour in words, probably our solution is too complex.

[Dragomir-Ivanov]

In my opinion, force update via HTTP call should be counted as "update" even if we don't change anything in the resource item. Think of touch semantics.

If one doesn't want that behavior, then he should make his own hook that can be executed last, that should inspect all relevant fields, and if no changes there, he can just make item=original and revert all changes, including any "updatedAt" fields, etc.

Sadly we can't modify HTTP response from hooks, so one can return 304 if no changes.

[smyrman]

In my opinion, force update via HTTP call should be counted as "update" even if we don't change anything in the resource item. Think of touch semantics.

OK, let's go with this for now then; it sounds easier to document.

The only feature I can think of where an early abort sounds useful, is for list PATCH operations, but that isn't a feature that we currently got. We can revisit it at that point.

The remaining question is then, is there a case for updating the E-tag between each hook (e.g. to check for changes between original and item), or is updating of the E-tag right before storing the resource sufficient?

[Dragomir-Ivanov]

I think updating right before storing is just fine for now. Will make a PR today or tomorrow.