ETag validity on composite resource

[Dragomir-Ivanov]

Lets suppose we have this request:

GET http://192.168.0.103:3001/api/clients/59a85126eaba9ebd0180ac55?fields=*,services,visits
If-None-Match: "11b22a28b07ddf482f8269050e9691d9"

If I execute this in a row, first time I get 200 then 304, which is as it is supposed to be.
But then I change some field values of services and visits behind the scenes.

Then retry the query above, I still get 304, although the result of the query IS CHANGED.

ETag comparison has its place on updating concrete resources, but I think we should leave generation of ETags for compound resources, and for resouseLists to be generated in a middleware function executed after rest-layer.

[rs]

Do you mean *,services{*},visits{*}? Because without {} you should just get the object ids.

This is a very valid point. What about computing a composite ETag?

cc @smyrman

[Dragomir-Ivanov]

@rs services and visits are bound to clients by schema.Reference and not schema.Connection so *,services,visits results in an arrays of services:[] and visits:[] attached to the returned clients item.

Composite ETag can't be cached in the DB, so we need to recompute it on the fly, compare with the supplied ETag and return 304 if they match.
Is all this work related to rest-layer at all, aren't we putting too many responsibilities on it? Other frameworks will do this in a middle-ware function executed after the frameworks.

I guess if we have some context ( but not Go context.Context) exported in the request, and we put some important values there to let the following middle-ware functions make decisions based on them.
I am against context.Context because it is hierarchical, and adding values in a child ctx doesn't propagate to parent ctx.

[Dragomir-Ivanov]

Okay, I think now I am understanding why middle-ware path is not considered here. In Go http.ResponseWriter is single shot, and once you Write() there is no turning back. That is why Alice middle-ware can be used only for modifying the http.Request object, but not the response.
Please confirm that my understanding is correct.

[rs]

Yes. That’s why you can set a custom ResponseSender on rest.Handler.

Computing a checksum of all the Etags shouldn’t be too expensive.

[Dragomir-Ivanov]

@rs Thanks, then custom ResponseSender is the way to go.

Computing a checksum of all the Etags shouldn’t be too expensive.

What do you mean by this? I thought the ETags are check-sums themselves. My thinking is computing ETag for composite and list resources will be the right way ( and is cheap, compared to network latency), and this might be done in the ResponseSender mentioned above. However we shouldn't enforce that, but maybe put an example for this particular case in the examples.

[rs]

Etag is an opac string. We can combine all etags of the (sub-)resources using the hash algo of our choosing to create a composite etag.

Why do you want to fix this issue outside of rest layer? It is a real problem that need to be addressed.

[Dragomir-Ivanov]

Well, now that it is clear we can't make post rest-layer middle-ware function to modify the response, we have no choice. I wanted because I have seen some Node frameworks doing in middle-ware functions, and it seemed self-contained problem.

Now, why would you want to combine all ETags, is it speed the only concern? Wouldn't be the same if you only calculate ETag (md5 hash) to the composite resource response? Combining multiple-ETags seems more work.

[rs]

Summing n string is less expensive than summing n maps with arbitrary number of fields (and subfields) stored as interface{}. We could checksum the JSON representation, it would be in-between in term of complexity but it would also be too late and tie the Etag computation to the representation which is handled by the rest package instead of resource package.

Functionally, I agree, both approaches are equivalent.

[rs]

Just looked at the code. Accessing the Etag during projection evaluation might be tricky actually. I think query.Projection.Eval should return a list of resources used during the evaluation for instance, so the caller can do additional work. I means that Etag comparison won't be able to happen before projection evaluation.

We have the same problem with If-Modified-Since and Last-Modified actually. In this case, we should return the min(resource, sub-resources).

[Dragomir-Ivanov]

What summing algorithm do you propose for Etags? Append all Etags and then do final md5 on the whole byte array, or XORing them all?

[rs]

I would go for md5. The xor route would be interesting but would require to either guarantee all Etags are same length or use padding. All rest-layer Etags are currently same size but it could change in the future or we may want to support pre-existing Etags with different format.

[smyrman]

Will the md5 hashed Etag be set in the header, while the raw MD5s are made available as _etag in each resource? Will the original Etag be available as a different header? How does the system respond if a resource is missing an E-tag, e.g. as discussed in rs/rest-layer-mongo#20.

One -case to be aware of:

1. A user fetches a resource with some other resource embedded GET http://192.168.0.103:3001/api/clients/59a85126eaba9ebd0180ac55?fields=*,services,visit
2. A user try to update the root resource if it has not changed PUT http://192.168.0.103:3001/api/clients/59a85126eaba9ebd0180ac55 If-Match X (X retrieved via 1)

[rs]

One solution would be to have a composite Etag with a separator like <root resource etag>-<composite projection etags>. The full Etag would be used by browser for caching, but we could strip the second part when the item is edited.

[Dragomir-Ivanov]

I think it is best if each resource brings its own _etag field just like in the itemList case.
Then we will have ETags to update freely the root resource or each of the projected resources.
I guess we will need to supply _updated as well.
Also when PUT/POST-ing a resource, we can look for If-Match HTTP header, if non we can fallback to trying to find _etag field within the resource, and trying to use that. This will not help browser caching, but will prevent concurrent resource update, which is more important.

[rs]

I’m not a fan of poluting the payload with metadata.

[Dragomir-Ivanov]

Okay, but we are polluting the payload in itemList case, aren't we? Adding _etag in the referenced resources as well would service the same purpose. Pollution of every kind is not wanted, but can't we make this configurable, or better supplying enough of the data, so custom ResponseFormatter will do the job.

[rs]

What’s wrong with the composite etag described above?

[Dragomir-Ivanov]

There is one particular use-case, where you get root resource, and a referenced resource item/items. Then you want to PUT/PATCH a referenced resource item. In your proposal, we can't do that, because we don't have the ETag for this individual item, but just for the root resource and for all projected resource items Etag combined sum.
If we want to update a referenced resource item then we will need to GET it again, then PUT. Not needing doing this is nice optimization, and will be in line of itemList case, where we have array of items, each with _etag field. In the root resource we also have array of referenced resources items, but without _etag field..

I understand that all this is optional, so maybe the best place to do this formatting is in the ResponseFormatter so the user can overwrite it.

[smyrman]

I suppose we have three or four cases for the composite e-tag:

• list view : only composite etag ("root item?")
• list view with embeds : another composite etag
• detail view : only root item etag
• detail view with embeds : root item and conposite e-tag

I suppose this will let you update the root-resource without a data race, but not an embedded sub-resource, e.g. if you want to fetch all resources through one big request with embeds, and update individual resources later without a re-fetch.

I’m not a fan of poluting the payload with metadata.

Neither am I. However, as @Dragomir-Ivanov points out, the rest package already include an "_etag" field in some cases. Why not do so consistently, and let that field be used for updates, and the header value E-tag for GET?

[smyrman]

Btw, making the header values e-tag of format <root resource etag>-<composite projection etags> when applicable is still nice in order to make it less confusing to the user why the E-tag in the header and payload sometimes do not match; at least he can see that the header E-tag actually includes the resource E-tag, and that it's of a longer format.

[Dragomir-Ivanov]

@rs Any thoughts on "polluting" the sub-resource payload with _etag?

[rs]

I have no better solution to propose so let’s do that :/

[Dragomir-Ivanov]

I was tinkering with the Etag for the composite resource, and it turned out that during the resource and sub-resource projection evaluation different parts of sub-resource projections can come from DB at different times, because some sub-resources are obtained with go-routines. This is a problem because for the same request, sometimes sub-resources can come in A,B,C order, but for other times they can come in B,C,A order, thus using md5.Hash() function producting different composite resource hasg/ETag, for exactly the same result. @rs Can you rethink your decision on using XOR of all ETags instead. It will produce the same result no matter the order.

[smyrman]

Can you rethink your decision on using XOR of all ETags instead.

I suppose that means that rs/rest-layer-mongo#20 would need to change to a compatible format?

sometimes sub-resources can come in A,B,C order, but for other times they can come in B,C,A order,

Have you actually observed this? From me just skimming the code, it looks like the order is pretty much guaranteed (unless they come in a random order from the Storer layer).

Expanded references are inserted at a given map field location, so evaluation order should not matter.

• rest-layer/schema/query/projection_evaluator.go

  Line 95 in 2c4c847

  res[name] = v

Connections are inserted at a given slice index:

• rest-layer/schema/query/projection_evaluator.go

  Line 121 in 2c4c847

  if payloads[i], err = evalProjection(ctx, pf.Children, payloads[i], validator, rbr); err != nil {

[smyrman]

I suppose it is true that they can come from the DB at different times, but does that matter?

[Dragomir-Ivanov]

Maybe I am not understanding the code enough, but for composite resource's ETag we will need to hash with MD5: root resource ETag, sub-resource A ETag, sub-resource B ETag, etc. We will need to apply the MD5 hash in the same order, for us to have the same composite-resource ETag. We will need ether store&compute this hash after all sub-resources have been obtained from storage, or use algorithm where ordering doesn't matter (XOR).
@smyrman The snippets above seem to be executed by separate go-routines, so there is no guarantees about their scheduling, and which finishes when.
I may be wrong, but that is my understanding for now.

[smyrman]

We will need ether store&compute this hash after all sub-resources have been obtained from storage, or use algorithm where ordering doesn't matter (XOR).

Ok, I follow you now. I was assuming you would do the first, compute the hash after all sub-resources have been fetched. If you want to do the incremental hash while you are fetching resources, then yes, you would need to use another algorithm than MD5.

[smyrman]

From me just skimming the code, it looks like the order is pretty much guaranteed (unless they come in a random order from the Storer layer).

I was thinking about the order being consistent in the final JSON response, not in terms of evaluation order, which is what you ask for -- sorry for the confusion.

[rs]

In the light of this, xor is the way to go. We can handle different sizes using padding when necessary.

[Dragomir-Ivanov]

Before proposing any code, lets try to agree on a way forward with this. This is what we currently have in rest-layer:
When returning itemList via ResponseFormatter FormatList we get _etag field unconditionally.
This query GET http://192.168.0.103:3001/api/clients?limit=1&fields=address

[
  {
    "_etag": "W/\"a2fef762819ea9e5b27855f6a11c534c\"",
    "address": "Bulgaria, Ruse, David street 89123"
  }
]

So we are already polluting the item, and there is no mention we wanted _etag at all. Imagine we are returning the whole item, make some changes, then wanted to PUT it back we should remove _etag manually from the body & compose our HTTP PUT request header If-Match with its value. Returning _etag for items doesn't help with Browser caching in any way.

Now what if, when PUT/POST an item we check for HTTP Header If-Match or a presence of _etag in the body. That way we will avoid composing custom HTTP header, and will not need to remove _etag if we wanted conflict-free update.

What about requesting _etag presence at all in the GET query at all:
GET http://192.168.0.103:3001/api/clients?limit=1&fields=address&etags=1
That way we will present the user actual means of controlling that _etag pollution.

Then for composite ETag, if _etags are present we can MD5 just them(as an optimization, however this will include the changes outside the current projection), or if not present we can MD5 the response body.

I will stop here for now, and we can discuss root item with sub-resources after we finish with this case.
@rs @smyrman Any feedback appreciated.

[rs]

Why a new query param and not just a projection field?

[smyrman]

Can I recommend that we splitting the issue in several parts to make it easier to solve?

Part A

I think the first problem to solve is GET when a referenced objects change so that we can reliably use the E-tag to check if resources are up-to-date when embedding sub-resources. E.g.

1. http GET 'http://192.168.0.103:3001/api/clients/<client-id>' fields=services If-Not-Match:W/"<db-etag>" where services is a sub-resource and/or reference that we want to expand.
2. A linked service change.
3. http GET 'http://192.168.0.103:3001/api/clients/<client-id>' fields=services If-Not-Match:W/"<db-etag>" cached by the browser, even though the JSON is different 😢

@rs idea to prefix with an MD5 could help resolve this regardless if sub-resources are polluted with _etag fields or not.

We can have an etag that is composed of two part, one which is the etag of the root object (as stored in the db) and another which is the compound etag (like checksum of the response).

E.g. "<MD5-sum-of-payload>-<db etag>" (W/ prefix removed as the E-tag is now strong). On POST/PUT/PATCH requests with If-Match, the server will use the <db-etag> only, as @rs describes.

For the list view, we change the implementation to let the header E-tag be a MD5 sum of the entire payload (also in case of HEAD, which means we must do more work for HEAD requests).

This probably requires some interface changes, like moving skipBody from the ResponseFormatter interface, and inserting it in the ResponseSender one.

Part B

Why a new query param and not just a projection field?

We create a new issue to optionally include DB _etags for sub-resources and embedded refrences, either via a custom query parameter, or via some form of (special-cased) field-selection. Remember that _etag is not currently handled as a schema.Field at all.

[smyrman]

Part A is the original issue basically...

[rs]

+1

I would put the db etag first in your example.

[Dragomir-Ivanov]

Guys, I was hoping we would focus on the itemList case first. On one side it could give us some hints on how to resolve the composite item case, and on other it is somewhat easier case.

@rs In order to achieve _etag as a projection field, we need to include it in item.Payload structure. If we do that, will we include the _etag implicitly in the resource Schema or leave the user to define it.
schema.Readonly will not be enough, as user will surely try to pass _etag to subsequent PUT request.

@smyrman I fail to understand what <MD5-sum-of-payload>-<db etag> will give us. Does it give us better Browser cache hit rate, or just avoids us polluting the root item with _etag field. If latter is the case, please consider we are currently polluting all items in itemList case. If we are going to pollute anything, we better pollute in controlled manner(on/off) and consistently over the whole stack.

[smyrman]

Browser cache hit rate, or just avoids us polluting the root item with_etag field. 

Actually neither, it helps with better cache miss rate!

If you don't have either E-tag or no-cache headers, as you have discovered with clients, you get fake browser cache hits when you have embedded items. E.g. in our case where we have a "dashboard"-like resource with multiple "widgets" as sub-resources. If we add or remove or re-order "widgets" on the dashboard, the dashboard won't correctly refresh without deleting cache in the browser because the browser think it's up to date.

The composite E-tag with both the db-Etag and MD5 sum of the full payload would make the browser GET cache behave correctly while still allow for conditional POST, PUT and PATCH of the root resource to function, given you replace or remove embedded items.

[Dragomir-Ivanov]

Actually I am advocating in case of composite resource, although I wanted to avoid discussion about it right now, we would have one and only ETag for the whole resource body (root and sub-resource). Thus the HTTP Etag will be fingerprint of the entire payload. Not two sections of it.

Lets focus on the itemList case. I think after resolving this, the composite item will be resolved naturally.

[Dragomir-Ivanov]

@smyrman I think that two-part ETag is unnecessary. Perhaps if you can give me some concrete example, I can find it useful.

[rs]

With the two-part ETag you keep the opportunity to use the db etag part to perform conditional writes.

[smyrman]

For the itemList case, you just need an MD5 sum of the full Json payload (no db E-tag, as there is no relevant one to use), and everything should just work, so that's a good place to start.

The two-part is only valid for the detail view for now (when ID is given in the URL), and, as @rs mentiones, you can then do conditional writes with the same E-tag with the root resource, e.g. if I want to PATCH the "description" field of my "dashboard" only if nobody else have done so since my last read.

[Dragomir-Ivanov]

@rs I am also talking about the same thing, however I fail to see why:
ETag: <MD5 payload>-<db etag> is more than

ETag: <MD5 all item _etag >

{
   "_etag": "root item itag",....
  "sub-resource1" : {
      "_etag": "sub etag",....
  },
  "sub-resource2": {
      "_etag": "sub etag",...
  }
}

Keep in mind, that we already have polluted itemList case with items _etag. Doing it that way, we will have consistent behavior between itemList and composite item, where one will find ETag he needs for conflict-free update in the _etag field in the item.
If one wants to update the root item, he will use its _etag field, and will do it with no issues. In the meat time the browser will keep the cache for the entire projection ( root + sub ).

@smyrman There might be very well the case, when one extract a list of items, then wants to update one or more items from the list, without any re-fetch. So there are cases where _etag fields are needed. In order to calculate the composite ETag, one might use the MD5 of all _etag fields in the list as an optimization. As I said before, that is Strong ETag, because will consider the updates outside the current projection.

[rs]

Let's imagine you have the following projection title,description,author{name}:

{
    "title": "A title",
    "description": "Some description",
    "author": {
        "name": "John Doe"
    }
}

Because author is a sub-resource, you can't just give the root (db) etag, you have to add the content of author so if the author name change you get a different etag. So as defined, you would MD5 the whole response instead of trying to get individual etags, which by itself, could be your final etag.

But now you want to PATCH the description in a CAS way. Normally you would use the Etag, but if it is an MD5 of a certain projection, we lost the ability to perform conditional writes. So using If-Match: MD5(projection-result) would always fail.

If we return both the root (db) etag and the MD5 of the response like W/"<db-etag>~<md5>", when the client use that value with If-Match, we can remove everything after the ~ and use the first part to perform the conditional write.

Said otherwise, we don't break one existing feature to fix the caching.

[Dragomir-Ivanov]

I totally agree with this, however what I am saying is this. Following the already implemented logic in the itemList case we have this projection:

{
    "title": "A title",
    "description": "Some description",
    "_etag": "123",
    "author": {
        "name": "John Doe",
        "_etag": "345"
    }
}

Trying to update/PATCH the root resource we supply the root _etag: If-Match: "123"`.

All I am proposing is to extend the current logic we have for itemList to composite resources that is all.
Along with ability to signal if we want the addional _etag pollution or not ( via HTTP Header, query parameter, projection field, etc. )

Browser caching will not be compromised, because the HTTP ETag header will be fingerprinf of the whole HTTP body ( root + sub-resources ).

Don't you think that is more flexible, as one can patch either to root resource, or any of the sub-resources without refetch, having already their _etags.

[rs]

Do you think it is practical to use the etag of a projected sub-resource to update it? In your example you don't even have it's ID and you would have to guess its path.

The item list use-case is already kind of stretched, but at least it's part of the same resource.

I think the main use-case is to use the standardized etag to perform conditional mutations without having to pick the etag into a custom field of the payload.

[smyrman]

@smyrman There might be very well the case, when one extract a list of items, then wants to update one or more items from the list, without any re-fetch. So there are cases where _etag fields are needed.

I vave haver disputed that, but this is already implemented. I am all for embedding more _etag fields, and get better control of it, but maybe not as part of this issue?

For this issue there is two cases:

1. The itemList view get a strong MD5 from the full response body, no other changes. This could be the first PR, as there seam to be no outstanding discussions related to this.
2. The single item view get a strong MD5. We can not easily know if we have embedded fields or not in this view at the Formatter/Sender, so here we need to either allow embedding of the DB _etag field as you suggest @Dragomir-Ivanov (for the root resource only to retain current functionality), or we need the two-component tag that @rs suggest. Eithe would work, but the two-component one don't change the current API usage workflows.

Do you think it is practical to use the etag of a projected sub-resource to update it? In your example you don't even have it's ID and you would have to guess its path.

The example from @Dragomir-Ivanov might not be complete, but if you embed the full sub-resource including the ID, then having the E-tag would be useful. Anyway I suggest we create a new issue / PR to address embeding of _etag in subresources, and to better control the inclusion of them in the payload, because I think this requires a bit more thought-process, not to mention the code changes. I am not against it, if I come across that way.

Does this make sense?

[Dragomir-Ivanov]

@smyrman The first post in this issue is about ETag on a composite resource, so I personally think this thread is good for discussing that matter. There is no code yet, nor PR result from this ( but I am willing to do the necessary changes when we settle on a solution ).

1. Also we already have code in master I did recently about ETag HTTP response header on itemList case. It is not strong MD5 sum from the body, but this easily can be achieved, and I don't have any objections on it (as we are all on the same page for this), and I can send PR for this right away.

2. I guess we have some misunderstanding / disagreement here. I still fail to understand what two component _etag will bring us. With including _etag in the item body we achieve consistency like using/updating the items will be the same no matter where they came from: itemList, single item ( with or without sub-resources), and sub-resource. All of them will have _etag in their bodies and their update procedure will be the same. With two-component ETag, we will have different update procedure for the root item, but the others 2 will rely on the embedded _etag field.

When supplying the _etag on PUT/PATCH rest-layer can use it to ensure conflict-free update ( as it came from If-Match ) and prevent its propagation to the Storage.

I understand your concern about not braking the API usage, but if we are to brake something, better be before v1.0. In a single root item case(no sub-resources), we can have the HTTP response ETag header to be equal to the item's _etag field

[Dragomir-Ivanov]

Keep in mind that in PUT/PATCH case, the user will need to set the If-Match HTTP header, and not the browser.

[rs]

Sure, but the HTTP spec says that you are supposed to use the value of the Etag header. If we don't follow the HTTP spec, we should not use the If-Match header because it becomes confusing.

Again, I don't like having _etag in the payload. If we can have it only on demand I would sleep better, and if we can make the main use-case of etag (CAS on single resource) work without this extra field it would be better.

[Dragomir-Ivanov]

@rs That was my idea the whole time. We should make this _etag pollution on demand ( via query parameter just like total=1 for X-Total, or HTTP header X-RS-Etag, or even we can we can enable/disable it via API, index.enablePayloadEtags(true).

Now are you talking about single resource without sub-resources or with sub-resources.

1. In case without sub-resources, HTTP ETag will be the fingerprint of the resource ( i.e. the same as not returned _etag ).
2. In case with sub-resources, HTTP ETag I guess we can use the two-part ETag. So updating the root resource will be possible, and the whole projection can be cached by the browser. Hope this two-part ETag is not too confusing for the users. What is your proposal for the format ETag: "[root item etag]"-"[body md5 sum]", with a dash between them and quotes around each hash sum?

[smyrman]

Now are you talking about single resource without sub-resources or with sub-resources.

In the ResponseFormatter/SendFormatter this is currently an impossible call to make, as the peojection is already applied. To keep both simpler code and a consistent user experience, it might be better to always go for case 2 and use the two-part ID.

[smyrman]

with a dash between them and quotes around each hash sum?

That breaks the spec, does it not?

[Dragomir-Ivanov]

Yes, maybe it breaks the spec. It will be ETag: "[root item tag]-[body md5 sum]".
@smyrman What do do you think _etag enable switch be?

[rs]

It does not break the spec and is not confusing to the user. The fact that it’s a composite etag is opaque to the user. The user still send the full etag, it is rest layer which choose to only use the first part for mutations.

[rs]

And I agree with @smyrman, we should start by just fixing the caching bug using composite etags. Let’s handle etag switch in a different issue.

[smyrman]

It does not break the spec and is not confusing to the user. 

Sorry, I was only reffing to the quotes around both pars - we should have only one set of quotes.