Skip to content

"method" does not directly constrain HTTP methods #284

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

Closed
wants to merge 1 commit into from

Conversation

handrews
Copy link
Contributor

@jdesrosiers this is an alternative to #280 that (I think) clarifies
the text to match your interpretation. Here is the diff between #280 and this:

MacBookPro:json-schema-spec$ git diff all-methods most-methods
diff --git a/jsonschema-hyperschema.xml b/jsonschema-hyperschema.xml
index bf844e2..bbaa0aa 100644
--- a/jsonschema-hyperschema.xml 
+++ b/jsonschema-hyperschema.xml
@@ -885,16 +885,17 @@ GET /foo/  
                     </t>    
                     <t>         
                         Despite being named after HTTP's GET and POST, the presence,-                        absence, or value of this keyword does not impose any constraints                        
+                        absence, or value of this keyword does not directly impose any constraints               
                         on either the protocol or method used to interact with the remote resource.          
                         In particular, the same Link Description Object may be used
                         for multiple protocol methods.
                     </t>    
                     <t>     
-                        For protocol methods whose request format is derived from
-                        the target representation, if "method" is "post" then
-                        <xref target="schema">"schema"</xref> and
-                        <xref target="encType">"encTYpe"</xref> SHOULD be ignored.
+                        If "method" is "post" then <xref target="schema">"schema"</xref>
+                        and <xref target="encType">"encType"</xref>, if present, 
+                        MUST be used to construct any request payload.  This indirectly 
+                        constrains the available protocol methods to those semantically 
+                        compatible with this restriction.
                     </t>
                     <t>     
                         Values for this property SHOULD be lowercase, and SHOULD be compared case-insensitive. Use of other values not defined here SHOULD be ignored.

@awwright I'm hoping to get a response from @jdesrosiers to my last major comment on #280, but I think we're getting pretty close to the point where you'll need to either pick that PR, this one, or submit an alternative of your own if neither of these match the current intent.


"method" already documented that "method": "get" does not constrain
the LDO to GET. Clarify that "post" also does not directly constrain
the LDO to HTTP POST, although the semantics of "schema" and "encType"
may have that effect in practice as they MUST be used for the
request payload if present.

Aside from that indirect constraint, an LDO can be used with multiple methods.

To clarify non-HTTP usage in generic terms, also add some wording
about request formats being defined in terms of the target
representation or not. This should provide more guidance on how
to use both "schema" and "targetSchema", independent of URI scheme.

"method" already documented that "method": "get" does not constrain
the LDO to GET.  Clarify that "post" also does not directly constrain
the LDO to HTTP POST, although the semantics of "schema" and "encType"
may have that effect in practice as they MUST be used for the
request payload if present.

Aside from that indirect constraint, an LDO can be used with multiple methods.

To clarify non-HTTP usage in generic terms, also add  some wording
about request formats being defined in terms of the target
representation or not.  This should provide more guidance on how
to use both "schema" and "targetSchema", independent of URI scheme.
@handrews handrews added this to the draft-next (draft-6) milestone Mar 26, 2017
@jdesrosiers
Copy link
Member

Yes, this is generally what I had in mind, although I think it needs re-wording.

First of all, "MUST" is too strong. "SHOULD" is more appropriate.

I don't think describing this concept as constraining methods is the right way to view it. I would say that using `"method": "post" implies sending data to an executable resource. It's up to the protocol to determine how to do that. With HTTP the way to do that to use POST. I wouldn't consider that constraint.

@handrews
Copy link
Contributor Author

@jdesrosiers If it's just a should I know I'm going to ignore it entirely, because as far as I can tell it's either superfluous (the link relation type already indicates this kind of interaction), or it's an annoying duplication (having to define every "collection" link twice).

@handrews
Copy link
Contributor Author

@jdesrosiers I know my last example didn't work well, but "collection" is one of the most common link relation types in REST (either used explicitly, or assumed through documentation in APIs that don't properly use links). And it is both executable (item creation) and non-executable (viewing, filtering, potentially bulk updates with PUT or PATCH).

If our guidance doesn't fit the most common (other than "self") link relations without pointless extra work, I think that's a problem.

@jdesrosiers
Copy link
Member

If it's just a should I know I'm going to ignore it entirely

As I understand it, SHOULD is a very strong requirement. It's not something you ignore because you disagree with it or it's inconvenient. You have to have a very good reason and even then you have to be aware that your implementation may not inter-operate with other implementations.

@jdesrosiers
Copy link
Member

If our guidance doesn't fit the most common (other than "self") link relations without pointless extra work, I think that's a problem.

It's still debatable that this causes "pointless extra work", but I think that is off topic here. The point of this PR is to clarify my interpretation of the spec. That is the way I interpret the spec as it is now. I now prefer the compromise proposal over this anyway. Minor changes would be necessary, but I think it would be worth it.

@handrews
Copy link
Contributor Author

As I understand it, SHOULD is a very strong requirement. It's not something you ignore because you disagree with it or it's inconvenient. You have to have a very good reason and even then you have to be aware that your implementation may not inter-operate with other implementations.

@jdesrosiers oh wow, sorry, I flubbed that. It was more of a facetious "this should be a MUST" argument that needed about three smileys and another paragraph of context. Sorry about that, I totally recognize that anyone would read it as "I'm taking my marbles and going home" which is super-counter-productive

@jdesrosiers
Copy link
Member

@handrews Thanks for clarifying :-)

@handrews
Copy link
Contributor Author

I think we all agree that this PR is not the path forward, and the main discussions are in #280 and #290 so I'm going to close this one to reduce clutter.

@handrews handrews closed this Mar 31, 2017
@handrews handrews deleted the most-methods branch August 27, 2017 23:02
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

Successfully merging this pull request may close these issues.

4 participants