Week of Feb 16#436
Conversation
- A bit of clarity around xref <-> normal resource processing - modify resource processing logic to handle the case where "versions" is present w/o any hint as to whether a separate default Version should be created. Adjust resource.md with more samples Signed-off-by: Doug Davis <duglin@gmail.com>
| without any indication from the client as to which Version those | ||
| attributes need to be applied to, rather than creating one, the assumption | ||
| is that one of the Versions in the `versions` collection is that Version. | ||
| - If neither the `versionid` nor `defaultversionid` are present, then the |
There was a problem hiding this comment.
I think this bullet needs to be updated to include the versions collection. (If there's just a versions collection, it doesn't need to generate a new versionid.)
There was a problem hiding this comment.
This bullet will never be processed if there's a "versions" collection w/o versionid & defaultversionid though - that's why I purposely didn't include it. See the previous bullet about stopping processing.
Maybe I need to clean this up more...
There was a problem hiding this comment.
It makes it hard to read when sometimes the "stopping" is performed by "otherwise", and sometimes by an instruction within the previous bullet.
If you just change this to "Otherwise, the server must" then I think it's clearer that this is picking the default value from multiple steps and each one stops the following ones.
| - If neither are present, then the server MUST generate, and use, a new | ||
| [`versionid` value](#version-ids). | ||
| - Otherwise, if the `versions` collection contains at least one Version, | ||
| then all of the Resource's default Version attributes MUST be ignored |
There was a problem hiding this comment.
I don't think this is right - or resource.md is wrong. See the "Create Resource with versionid and Versions" example, where v0 is only mentioned in the resource's default version attributes, but it is in the result.
I think we do want to ignore all the resource's default version attributes if there's a version in the collection and meta.defaultversionid refers to a version within the collection.
There was a problem hiding this comment.
Again, the "otherwise" is critical. If Resource.versionid is present then this bullet will never be processed.
There was a problem hiding this comment.
I think it's more that I hadn't clocked the scope of this change is only within "determine the default version".
| request, then it MUST be used. Also see | ||
| - If a Resource-level `versionid` attribute is present in the request, | ||
| then its value MUST be used. | ||
| - Otherwise, if a `meta.defaultversionid` attribute is present in the |
There was a problem hiding this comment.
I don't think I'd make this an "otherwise" - i.e. we shouldn't just entirely ignore meta.defaultversionid because there's a versionid. I think yesterday I suggested we should check them for consistency - but I may be wrong. (The fact that I'm finding it hard to remember what I said yesterday is somewhat worrying in itself, in terms of the complexity of these rules...)
There was a problem hiding this comment.
If we check for consistency, we should throw an error if they're inconsistent, right?
I'm generally in favor of that, but I believe there was a preference towards throwing less and ignoring values in the order of relevance, as the text currently suggests.
There was a problem hiding this comment.
The "otherwise" is actually really important because it's an if-then-else thing. If Resource.versionid is present then meta.defaultversionid IS ignored for the purpose of trying to figure out which versionid should be used as the "default version as seen before the op started". Yes meta.defaultversionid might be used for other purposes later, but not for this specific task/step of steps.
There was a problem hiding this comment.
Okay, just for processing the default resource attributes, I agree.
| is that one of the Versions in the `versions` collection is that Version. | ||
| - If neither the `versionid` nor `defaultversionid` are present, then the | ||
| server MUST generate a new unique [`versionid` value](#version-ids). | ||
| - If a Version with the target default `versionid` does not exist then a |
There was a problem hiding this comment.
I think this should outdented, because it's not about determining the version to be used as the default version ID.
Signed-off-by: Doug Davis <duglin@gmail.com>
| - If `versions` is missing, or empty, then the server MUST generate a | ||
| new unique [`versionid` value](#version-ids) and create a new Version | ||
| using that value, with the Resource-level attributes. | ||
| - If `versions` is present with at least one Version, then the |
There was a problem hiding this comment.
Would it be worth repeating here that the default version will be the last one based on alphabetical order?
There was a problem hiding this comment.
Perhaps, IF this bullet mentioned "default version", but notice that it doesn't. I didn't think mentioning "default version" here helped (in fact I think it might confuse things) because regardless of which Version in "versions" ends up becoming the default one doesn't change the fact that we're ignoring Resource.* attributes. And calculating which Version in "version" ends up being the final default Version isn't really part of this discussion. Make sense?
|
Approved on the 2/19 call |
is present w/o any hint as to whether a separate default Version should
be created. Adjust resource.md with more samples