mamund / hal-forms

backward-compatible extension for HAL to support dynamic forms at runtime.
MIT License
30 stars 18 forks source link

Feedback on options attribute #76

Open odrotbohm opened 3 years ago

odrotbohm commented 3 years ago

While working on the HAL FORMS implementation in Spring HATEOAS and collaborating with @toedter on the HAL Explorer there are a couple of observations that I'd like to share here.

Form field cardinality

A fundamental challenge in producing requests from filled out forms is deciding whether to send the value(s) selected as scalar value or array of values. I was under the assumption that the presence of any of the …Items attributes indicates an array to be used, but Kai rightfully pointed to the spec unconditionally defaulting minItems to 0 and maxItems to unbounded. The implementation in HAL Explorer currently inspects maxItems for the value of 1 and uses that as an indicator that we deal with a single-select and send the selected value as scalar one in e.g. a JSON request. That causes a few challenges:

  1. Servers cannot express the requirement of a single valued collection in any reasonable way.
  2. The processing model is quite complicated: to decide whether to submit the value as array or scalar item we need to inspect the attribute and apply some implicit knowledge about the value.

Suggestion

That would allow clients to decide about the way the selected value(s) to be submitted based on the presence of the attributes alone. minItems/maxItems missing completely would indicate single select and scalar submission.

Remote options response format

In an example project I returned HAL for the response of a resource pointed to by a link attribute. I read this section of the spec in a way that fundamentally, the client should be able to work with any collection of scalar values or value/prompt pairs that the client can be made to work with. I.e. if the server returned a HAL flavored { "_embedded" : { "field" : [ … ] }} and the client generally understands HAL, it should consume the field array (more precise: just grab all keys within _embedded and flat map concatenate them). Kai argued, that it's not quite obvious that this should be supported as the current working speaks about arrays and collections as the linked to examples only show raw JSON arrays.

Suggestion

Clarify that the exact format of the collection is defined by the media type returned by the server and maybe also show an example of something other than raw JSON. Remove the sentence that type should be set to application/json or text/csv. This would e.g. allow also using resources that return other media types as long as they adhere to the general format (scalar values or value/prompt pairs).

I think that, HAL FORMS being an extension of HAL, we should make sure that the interaction models should also describe, how to behave with resources using HAL. Currently, under strict interpretation of the FORMS spec, we cause quite a bit of friction in terms of representations that have to be supported by servers. I.e. if I have a HAL based API and would like to extend that using HAL FORMS, I ideally would like to be able to use already existing resources and representations, or – if I need to create new ones – at least stick to the same media type for those resources so that clients already understanding by base media type allow seamless interaction with those resources.

toedter commented 3 years ago

Related to "Remote options response format": HAL Explorer currently interprets the content-type of the response. If the conten-type is the one of HAL or HAL-FORMS AND the response has a top-level _embedded property, then the options are filled from { "_embedded": { "any field name": [ ... ] }. An example response is in my test data: https://github.com/toedter/hal-explorer/blob/master/test-data/shipping.hal.json. I would agree with @odrotbohm to specify the expected structure.

evert commented 3 years ago

Form field cardinality

I still think it would be helpful to just add a multiple property. Defaults to false, if set to true, it means the value should always be encoded as an array. I had the same issue with Ketting, and this seems like an easy solution.

Remote options response format @toedter : If the content-type is the one of HAL or HAL-FORMS AND the response has a top-level _embedded property, then the options are filled from [...]

I agree with the general premise that this should work better with existing HAL documents, but I think it's a mistake to overload _embedded for this.

For us at least _embedded has no semantic meaning. It's a mechanism that allows a server to pre-populate a cache. A client does not consider the HAL document semantically different if there are _embedded resources or not. The only difference is that it might need to do fewer HTTP responses.

Our client will actually inform the server to embed specific resources (based on a link relationship) using the Prefer-Transclude header.

This interpretation is at least in-line with the following recommendation from the core HAL draft:

https://tools.ietf.org/html/draft-kelly-json-hal-07#section-8.3

The "hypertext cache pattern" allows servers to use embedded resources to dynamically reduce the number of requests a client makes, improving the efficiency and performance of the application.

Clients MAY be automated for this purpose so that, for any given link relation, they will read from an embedded resource (if present) in preference to traversing a link.

To activate this client behaviour for a given link, servers SHOULD add an embedded resource into the representation with the same relation.

Now I'm not saying that you are explicitly wrong in overloading the _embedded property for other purposes in your servers or clients, but I am saying that this is not how everyone in the community interprets _embedded, and this behaviour should definitely not make its way back into HAL-Forms.

However, I proposed a solution in #74. I believe that this solution (which is based on _links, not _embedded) can also fulfill your use-case without relying on any non-standard magic. In your case you can just point to an external HAL resource, and specify which rel you want to use as the list.

Then if you choose to embed those resources anyway, that's entirely up to you.

toedter commented 3 years ago

Thx @evert, I will rethink this in the next release. I would be interested in a proposal for a HAL-based options response, to make it possible for generic clients (like HAL Explorer) to interpret it correctly.

evert commented 3 years ago

@toedter i would be curious to hear your thoughts on #74 and if it would solve your problem!

odrotbohm commented 3 years ago

I agree with the general premise that this should work better with existing HAL documents, but I think it's a mistake to overload _embedded for this.

For us at least _embedded has no semantic meaning. It's a mechanism that allows a server to pre-populate a cache. A client does not consider the HAL document semantically different if there are _embedded resources or not. The only difference is that it might need to do fewer HTTP responses.

Our client will actually inform the server to embed specific resources (based on a link relationship) using the Prefer-Transclude header.

This interpretation is at least in-line with the following recommendation from the core HAL draft:

https://tools.ietf.org/html/draft-kelly-json-hal-07#section-8.3

I pretty fundamentally disagree with this. While _embedded can be used for that, it's not specified to be only used for that. The relevant spec section is here, and there literally is no other way to represent collections in HAL other than embedding. In fact, the spec clearly shows usage of HAL to represent collections that way and that example is not implementing the cache pattern. I think there is a reason the pattern is listed very late in the document as "recommendation".

That said, options are a preview of a dedicated resource. In fact, you even asked for them to feature links as first class citizens for values. I.e. remote options are likely to point to other resources, for which the resource backing the URI pointed to in link is a list of previews.

I think we can avoid the entire discussion by not prescribing how the remote options are supposed to be processed but only specify that the should be processed according to the returned Content-Type header (I think that's already in the spec) but avoid the impression that the given examples of a raw JSON array and CSV are exhaustive.

evert commented 3 years ago

I pretty fundamentally disagree with this. While _embedded can be used for that, it's not specified to be only used for that. The relevant spec section is here, and there literally is no other way to represent collections in HAL other than embedding.

A collection can be represented by creating a list of links just fine. Also including their representations is a good idea, but optional. Our clients will typically get a collection + every item embedded on a first load, but only get the list of links on subsequent loads (wihtout embeds), because they typically only need to know if there were changes in collection membership and not fetch every item again. The fact that we need to include the body of every member at all is just to avoid HTTP per-request overhead and not without drawbacks.

But I've seen people treat _embedded different, and that's fine. My point was really not 'you're doing it wrong!', but rather.. we have a different idea about the semantic meaning, I know I'm not alone and I hope this can be considered.

I think we can avoid the entire discussion by not prescribing how the remote options are supposed to be processed but only specify that the should be processed according to the returned Content-Type header (I think that's already in the spec) but avoid the impression that the given examples of a raw JSON array and CSV are exhaustive.

I agree, and I feel that the inclusion of a rel property to use a list of link relationships on a remote resource (such as HAL) would both cover my use-case, and also work if you use _embedded the way you both do. The keys of both the _links and _embedded properties are a link relationship type.

odrotbohm commented 3 years ago

I agree, and I feel that the inclusion of a rel property to use a list of link relationships on a remote resource (such as HAL) would both cover my use-case, and also work if you use _embedded the way you both do. The keys of both the _links and _embedded properties are a link relationship type.

I'll leave another thought on rel on the other ticket.