Knotx / knotx-fragments

Fragments map-reduce processing using Graph flows, supplier and assembler.
https://knotx.io
Apache License 2.0
3 stars 5 forks source link

Support for HTTP PATCH/POST/PUT/DELETE #73

Closed tomaszmichalak closed 4 years ago

tomaszmichalak commented 4 years ago

Is your feature request related to a problem? Please describe. Knot.x HTTP Action is a generic Action implementation supporting HTTP GET requests. Other HTTP methods, such as POST/PUT/DELETE, are not supported. We anticipate absent Actions' implementations.

Describe the solution you'd like I would like to:

All Actions' implementations should be separate modules.

Additional context

marcinus commented 4 years ago

There is an open question: how many times should the schema be interpolated? The default answer would be once, but the following use case is also conceiveable:

The schema happens to be dependent on the result of the preceeding actions. Consider a configuration entry body = {payload.previousAction.retrievedSchema} If the retrievedSchema contains values that should be interpolated, the interpolation would have to occur twice.

The nesting could be repeated even more times. There is also another issue - when evaluating interpolation without an arbitrarily set number of iterations, an infinite recurrsion may occur.

Another approach would be to create a separate bodyProvider and assume that the body to be send with the PUT/POST request. This could be achieved via the behaviour mechanism (like CircuitBreaker) - the wrapping action would take the responsibility of constructing body and providing it in payload.

marcinus commented 4 years ago

As we're going to support sending body, we should also enhance Knot.x Server Common Placeholders to interpolate basing on Client Request's body. Open question - what content type should be supported? How should client request's body be mapped to properties if it is not in JSON format?

malaskowski commented 4 years ago

Maybe we could start with supporting only JSON Body. In other case we can choose one from options below:

marcinus commented 4 years ago

Wondering whether Content-Type header should be set OOTB to text/plain or application/json when body is provided. It could be overriden later.

marcinus commented 4 years ago

Another concern found: what is the best behaviour for the case when some key in provided JSON body becomes evaluated to an empty string? It's not possible to have such key in JSONObject (and there could be multiple). Possible solutions:

I'd choose the fourth option, but perhaps this should be configurable? It's a pretty exotic use case, but "silent-fail" option 1 could sometimes be of use.

malaskowski commented 4 years ago

I'd vote for the 4th option with improvement ticket to implement 1st as configurable in the future. If it is not possible to generate a request body, let's just not do the request. And definitely 1st should be not a default option, it might create a lot of confusing things to happen.

marcinus commented 4 years ago

Another features to consider: