Only parsed values#27
Only parsed values#27jaymode wants to merge 9 commits intopgomulka:compat_plugin_inside_rest_requestfrom
Conversation
There was a problem hiding this comment.
I like that approach too.
We tried sth similar previously in elastic@8770870
but I thought that the overall conclusion was that this spreads the compatibility changes to too many classes?
I personally don't mind sharing globalMediaTypeRegistry with plugins, but I am also happy with the approach in this PR.
@jakelandis do you have an opinion on this?
I can merge this and we could continue working on it on the main PR elastic#60516
| this.requestId = requestId; | ||
| this.acceptMediaType = mediaTypeParser.parseMediaType(header("Accept")); | ||
| this.contentType = mediaTypeParser.parseMediaType(header("Content-Type")); | ||
| this.formatMediaType = mediaTypeParser.fromFormat(param("format")); |
There was a problem hiding this comment.
we can possibly use this in sth like a getResponseMediaType method and be used together with accept media type. I think it should be done as a part of elastic#62294 later
| //TODO: USAGE_1 now that we have a version we can implement a REST handler that accepts path, method AND version | ||
| Version version = compatibleVersion.get(request.header("Accept"), request.header("Content-Type"), request.hasContent()); | ||
|
|
||
| Version version = compatibleVersion.get(request.getContentType(), request.getContentType(), request.hasContent()); |
There was a problem hiding this comment.
we could also add a getCompatibleVersion on a request. Thanks to this we won't need to extract the internals of RestRequest like getContentTYpe, getAcceptType hasContent etc..
We won't need to pass the plugin (CompatibleVersion interface) over to RestController as well.
I recall you re-working the code to avoid it. While working on this I came to realize that there is a bit of a blurry line in what is handled in the HTTP transport vs what is handled in the Rest controller. If we start to get into this then we should probably bring the distributed team into the discussion. |
2 participants
I took your PR and tried to turn my thoughts into code. Let me know what you think.