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

The http-request step needs to be updated #13

Closed
xml-project opened this issue Dec 31, 2016 · 18 comments
Closed

The http-request step needs to be updated #13

xml-project opened this issue Dec 31, 2016 · 18 comments
Assignees
Labels
standard step library Issues for the standard step library

Comments

@xml-project
Copy link
Member

The http-request response needs to be updated 

This is an aggregation of the discussion on "The http-request response needs to be updated" (Issue 170 of the xproc/1.0-specification)

Opened by: ndw on 2015-06-10, 10:21h

ndw said on 2015-06-10, 10:21h:

The current result is a c:response element with embedded possibly base64 encoded c:body element(s). If the response is multipart, we should probably now produce a sequence. Put the headers in the document-properties? 

On 2015-10-07, 14:27h: ndw added this to the XProc 2.0 LC milestone.
@ndw ndw changed the title Legacy from 1.0: The http-request response needs to be updated Legacy from 1.0: The http-request needs to be updated Sep 6, 2018
@ndw
Copy link
Collaborator

ndw commented Sep 6, 2018

Follow Adam’s lead from http://expath.github.io/expath-cg/specs/http-client-2/

Probably simplifying the return type for single entity bodies.

@xatapult
Copy link
Contributor

xatapult commented Sep 6, 2018

@Gertone is going to draft something

@ndw ndw transferred this issue from xproc/3.0-specification Nov 1, 2018
@ndw
Copy link
Collaborator

ndw commented Feb 5, 2019

Notes from the 05 Feb 2019 workshop:

  • JSON document as input? Yes, of course. Recursive maps.
  • Allow the 11 current attributes on c:request as options on p:http-request
  • Error if you specify an option in both places and they’re inconsistent
  • Review the http-request work that Adam did for EXPath
  • A simple body that isn’t a c:request is a single c:body
  • A simple body that is a JSON object with a “http-request”
    key with the value “request” is a complex request.
  • If the input is JSON, a detailed response is JSON
  • If the input is XML, a detailed response is XML
  • Change the ‘detailed’ option to take JSON/XML as values
  • Geert observes that he likes the work in EXPath except for
    the fact that the response is always a map, even in the simple
    case.
  • “Where do we say that for URI schemes (such as file: and ftp:)
    where a content type is not provided by the underlying request,
    the content type is implementation-dependent?”
    • If we don’t, we should.
    • Use the same algorithm as p:load/p:document
  • “The requirement to process the external subset comes from
    p:load, we probably don't want to impose that on all
    p:http-request calls. Need a way to control it?”
    • No.

@ndw ndw changed the title Legacy from 1.0: The http-request needs to be updated The http-request step needs to be updated Jun 10, 2019
@Gertone
Copy link

Gertone commented Sep 12, 2019

Finally getting my head around this again. Shyed away a bit before due to the complexity multipart might impose on simple requests. Just wondering. Would it be an option to have a "http-simple-request" that could be a mandatory step and a full blown "http-request" that does all the complex stuff multipart gives. But as a consequence has a more complex interface.

@Gertone
Copy link

Gertone commented Sep 12, 2019

The simple request step could limit the required verbs as well.
I am tempted to suggest a longer list than the one in that as per #42 anyway

  • GET, POST, PUT, DELETE, HEAD
  • add PATCH to that list to complete the restfull crud spectrum
  • add OPTIONS and TRACE to that list for informational / debugging reasons
  • this list is not in line with the function list in http://expath.github.io/expath-cg/specs/http-client-2/#functions, but we need not to completely align with that list since PATCH is not on it

@ndw
Copy link
Collaborator

ndw commented Sep 12, 2019

This division doesn't seem like a significant improvement to me. If we wanted to divide it into two steps, I'd be more inclined to have p:http-get and p:http-request steps.

@Conal-Tuohy
Copy link
Contributor

Rather than divide it into two steps, what about adding an option to control response formatting, alongside the other options status-only, override-content-type and detailed? e.g. multipart="false" could produce the simplified output for the common case of single part responses.

@xml-project
Copy link
Member Author

Interesting idea. What do you propose to happen when I have multipart="false", but the HTTP server returns a multipart response. That should be a dynamic error, shouldn't it?

@ndw
Copy link
Collaborator

ndw commented Sep 13, 2019

I haven't given this a whole lot of thought, but how about moving some of the request options onto the step itself; the fact that we have AVTs makes this a whole lot more practical for authors.

<p:declare-step type="p:http-request">
   <p:input port="source" content-types="*/*"/>
   <p:output port="result" sequence="true" content-types="*/*"/>
   <p:option name="serialization" as="map(xs:QName,item()*)?"/>
   <p:option name="href" as="xs:string"/>
   <p:option name="method" as="xs:string" select="'GET'"/>
   <p:option name="timeout" as="xs:positiveInteger"/>
   <p:option name="headers" as="map(xs:NCName,xs:string+)"/>
</p:declare-step>

Now simple GET requests don't require any c:request element. We can say that the source input is ignored for GET, that means authors don't even have to worry about how the input port is
bound on GET.

For other methods, if a c:request body is provided, it's interpreted for constructing the payload. If the input is not a c:request body, then it's used directly as the payload. If a sequence of inputs arrives, a multipart-mixed payload is constructed by default.

That also lets us simplify the c:request element:

<c:request
  detailed? = boolean
  status-only? = boolean
  username? = string
  password? = string
  auth-method? = string
  send-authorization? = boolean
  fail-on-timeout? = boolean
  override-content-type? = ContentType
  (c:multipart | c:body)?
</c:request>

I don't think any of this reduces the implementation complexity or the number of places where we have to check for consistency in the inputs, but I'm a whole lot more concerned about having a step that's easy to use.

@xml-project
Copy link
Member Author

Yes, that is the way I was thinking along. (With the difference of having a third map parameters on p:http-request for the options on c:request).

But one question:

Now simple GET requests don't require any c:request element. We can say that the source input is ignored for GET, that means authors don't even have to worry about how the input port is
bound on GET.

How do you do that. Input ports have to be connected, so you have to supply a p:empty for "GET" or "DELETE". Failing to do so is a static error.
What did I miss?

but I'm a whole lot more concerned about having a step that's easy to use.
IMHO that should be our number one priority.

@Conal-Tuohy
Copy link
Contributor

Conal-Tuohy commented Sep 14, 2019

One thing that concerns me about mapping multipart messages to an XProc sequence is that it doesn't seem to allow for nesting. A multipart MIME message may contain parts which are themselves multiparts.

Also I'm not sure how the multipart=sequence approach would deal with the different types of multipart message (e.g. multipart/mixed, multipart/alternative, etc). Would it still allow the different subtypes of multipart to be distinguished? This is where EXPath's approach is superior (if I understand it correctly), because it doesn't over-abstract from the underlying HTTP.

I use http-request a lot, in my work, with a fair variety of systems, which is why I think it's important that it should ideally be fully general; that is, able to handle any kind of HTTP request and response (which is why I also dislike the idea of a "white-list" of HTTP methods).

@xml-project
Copy link
Member Author

Conal-Tuohy said:

which is why I also dislike the idea of a "white-list" of HTTP methods

👍

@Gertone
Copy link

Gertone commented Sep 14, 2019

Well, the EXPath approach does cover the whole complexity. It is good, but it also adds a lot of complexity to the simple cases. That is one issue to cover.
Nested maps could allow for the nesting potentially required actually.
About the white-listing.. I brought it up here because #42 is labelled "consensus-reached". The way I see that: it is a mandatory set of methods an implementer is required to implement to be compliant. That is not necessarily as restrictive as you seem to believe

@Conal-Tuohy
Copy link
Contributor

Conal-Tuohy commented Sep 14, 2019

Regarding the whitelisting of methods:, this is a bit off-topic here, but just quickly: it's not that I think a white-list is in itself restrictive, but it does validate an implementation which is restrictive. I don't believe those restrictions are necessary; I don't believe that it's significantly more difficult for an XProc implementer to support any and all HTTP methods than it is to just implement support for the most popular 3 or 4. I think, though, that if the spec mandates a certain restricted set of methods, it will, in practical terms, have the effect of restricting at least some implementations to those methods.

@Conal-Tuohy
Copy link
Contributor

Regarding the extra complexity of EXPath's approach, can we discuss this with the aid of some example?
It seems to me that the common case, when I just want to access the actual entity returned by the HTTP server, in the step subsequent to the http-request, I could just use an XPath expression like .?body -- is it more complicated than that?

@xml-project
Copy link
Member Author

On editors call on 2019-10-03 we came up with this idea make response processing easier: The step get an option accept-content-type which take the same mime type syntax as content-types. If p:http-request returns result which does not match one of the listed content-types, an error is raised.
So: I you do not want a multi-part response you could use this option to make sure an error is raised, if the request return one. Or: If you know, that no multi-part response will result from a request, you can write a pipeline just dealing with a simple response without additional checks.

@xml-project xml-project added the standard step library Issues for the standard step library label Dec 22, 2019
@xml-project xml-project self-assigned this Jan 5, 2020
@xml-project
Copy link
Member Author

See pr #330

@xml-project
Copy link
Member Author

Addressed in pr #330

Closed as suggested in #330 (comment)

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

No branches or pull requests

5 participants