Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification needed: templates, key and defaulting #82

Open
odrotbohm opened this issue Jul 30, 2021 · 9 comments
Open

Clarification needed: templates, key and defaulting #82

odrotbohm opened this issue Jul 30, 2021 · 9 comments

Comments

@odrotbohm
Copy link

Template key specification

The specification of the _templates collection is a bit odd. It lists all properties, including the key property, when in fact – I think – key is not an actual property, but the key of the dictionary of all templates. From the spec, you could think that a _templates definition might look like this:

{
  …,
  "_templates" : {
    "default" : {
      "key" : "default",
      "contentType" : …,
      
    }
  }
}

The reason you can get to this assumption is that key is discussed on the same level as contentType et al. and among all properties that can be used within a single template entry. Does it make sense to pull that discussion up into 3.2 to avoid this confusion?

Template key defaulting

There's still room for clarification in the required naming for multiple templates. Assume you have a single, logical template foo. As per the spec, that needs to be named default. If that resource evolves to support a second template bar, would (should?) the list of template names change to foo, bar or stay with default, bar? The former might be disrupting to clients that look for default only. The latter – and that renaming in general – mask the logical name for the advertised first transition.

This always seemed a bit weird to me. What's the original intention behind that "if it's one, it's got to be named default" rule? I suspected that it was introduced to make it easy for clients to find "a default" template, but as the spec currently restricts the rules to the case of one template present only, it would be the single, canonical template anyway, wouldn't it? To convey those high-level semantics, I would expect using default to be of even greater significance in the case of multiple templates, although you could argue that with a stable key ordering, clients could generally assume the first entry to be "the default one". I am not sure how common it is to rely on ordering of the keys within a JSON map, though. If it's not common, then staying with default in the case of multiple templates available would kind of be required.

The current state of affairs complicates the implementation of the server as it's not obvious how to behave in case of multiple templates (see this ticket in Spring HATEOAS) for example. Anyone aware how other server libraries treat this?

@mamund
Copy link
Owner

mamund commented Aug 24, 2021

@odrotbohm :

yes -- this is a weak point in the spec. it was originally there because we wanted to start with just a single template in the response but allow for futures where multiple templates where returned. so, you're assumptions are correct.

what's a good alternative here? things we need to support (i think):

  • there is only one template offered -- what is it's keyname?
  • there are multiple templates offered but the call does not indicate which one to pick (missing arg) -- what template is returned?

there may be other important scenarios, too.

ideas?

@ecattez
Copy link

ecattez commented Jan 5, 2022

Hi,

I think default should not exist at all. It does not give any information about what it can or can not do. You have to check name, method, properties to understand what the template does. I think the template keys should in most case be domain driven.

For example, if we've got a template to confirm a payment, it could be named confirm-payment, being the only available template or not.

Is there a use case where default is obvious for a client ?

I'm working with HAL+Form in business projects and this default template is really painful. I've got a lot of resources with each its representation and I'm like "Oh yeah, in this representation, the default template does that" instead of "Of course confirm-payment just confirms a payment !".

Maybe default should be a boolean property of templates instead, like:

confirm-payment: {
  default: true,
  name: 'Confirm payment',
  properties: [...]
}

@mamund
Copy link
Owner

mamund commented Jan 5, 2022

yes @ecattez , this needs attention.

i'd like to make sure we don't break anything that's already out there, so first, i'd like to keep "default" (taking away is a breaking change, i think).

but maybe we can move this from "MUST" to "SHOULD" and then offer some guidance along the lines of @odrotbohm to help those curently depending on it.

something like this text:


The unique identifier for this template element. This is a REQUIRED element. For this release, the only pre-defined value for key is "default". If there is only one element in the collection the key value SHOULD be set to "default". Clients MAY ignore additional entries in the collection. If this element is missing, set to empty or is unparsable, this template object SHOULD be ignored.

@mamund
Copy link
Owner

mamund commented Jan 5, 2022

@odrotbohm

your comment about the documentation is spot on. i've never quite had the wright way to say that the collection key (JSON key) is what this is about, not a property named key.

surely this is a common problem, right?

what's a better way to write this up?

@odrotbohm
Copy link
Author

Summarizing, regarding default, what if we:

  • switch to "SHOULD" for default for the name of the template
  • indicate that, if only a single template is present that should be considered the logically default one regardless of name
  • for multiple templates present, clients that need a default template identified could look for one named default but should consider the first map entry the default one as fallback

Re keys: I think it's fine to talk about the key requirements right here:

The _templates element contains a dictionary collection of template objects. …

in a way discussed above and remove 3.2.2 entirely?

@odrotbohm
Copy link
Author

This issue just came up again. @mamund – Do you think we're far enough down the discussion to come to some level of consensus? I'll happily draft a PR.

@mamund
Copy link
Owner

mamund commented Jan 15, 2023

yes. let's sort out an agreement and make the changes.

feel free to draft a PR to get this rolling and let's see if we an close this out in a timely way.

thanks for this.

@odrotbohm
Copy link
Author

Would I submit changes to the index.adoc, index.html or both?

@mamund
Copy link
Owner

mamund commented Jan 15, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants