Introduce HttpResource service #50654
Comments
mshustov
added
blocker
Team:Core
|
Pinging @elastic/kibana-platform (Team:Platform) |
|
Should this be a sub-service of http.resources.add({
path: 'my-page', // server under '/my-page/'
fromRoot: true,
handler(){}
}); |
Let's look at HttpServiceSetup . If we want to have a common set of interceptors, auth/base path utils, etc. it sounds reasonable. However, we need to remove access to a plain router and provide HttpResource & HttpApi services instead. HttpResource service use-cases:To serve a static HTML page to bootstrap the Kibana app on the client.
HttpResourceSetup.registerEntryPoint: ({ path: string }) => void
http.resources.add({path: '/', validate:..., options: {auth:...}}, (context, req, res) => {
return res.app.render()
})to serve a dynamic HTML page.Examples: 1 2 http.resources.add({path: '/', validate:..., options: {auth:...}}, (context, req, res) => {
return res.app.render({
// will add CSP & 'cache-control': 'private, no-cache, no-store' headers
body: `<!DOCTYPE html> ...`
})
})to serve static assets from a directory.icons, images, fonts, etc. HttpResourceSetup.registerStaticDir: ({ path: string, dirPath: string }) => voidIt can be an internal API. We can enforce a convention for plugins to provide assets via the to serve dynamic assetsExamples:
http.resources.add({path: '/', validate:..., options: {auth:...}}, (context, req, res) => {
// will add etag header
return res.assets.custom({
body: '..',
headers: { 'content-type': 'text/javascript' }
})
}) |
|
A sub-service / API makes sense to me. I think it makes finding these features much more ergonomic. One thing that is a little inconsistent with these proposed APIs is that registering HTTP routes happens on a Alternatively, what if we changed the On the specific proposals above, I think they all make sense to me except the
These two usages actually are different enough that I think they should be separate functions. The first renders the Core bootstrapping HTML page, while the second renders a custom HTML document. The only thing they both do is include the standard headers (CSP, cache-control). I also don't think the function for rendering an HTML document should be in the Two APIs would make more sense to me: |
|
I'd say that I'm fine to duplicate helpers for |
What extra work would there be for plugins? Is this the problem with route wrappers with different router types that may have different arguments? Seems like it may be possible to create a wrapper that works for routers with different response factories: type GenericRequestHandler<P, Q, B, Method, RouterArgs extends any[]> = (
req: KibanaRequest<P, Q, B, Method>,
...args: RouterArgs
) => KibanaResponse | Promise<KibanaResponse>;
const wrapRoute = <P, Q, B, Method, RouterArgs extends any[]> = (
handler: GenericRequestHandler<P, Q, B, Method, RouterArgs>
) => GenericRequestHandler<P, Q, B, Method, RouterArgs> {
return (req, ...args: RouterArgs) => {
// do pre-route things here
// then run wrapped handler
return handler(req, ...args);
}
}This pattern could also be extended for wrapping the entire IRouter interface(s). Though of course, changing the existing wrappers to work with this is still more work. Is there something else about option (1) that creates more work for plugins? Even still, I'm ok with leveraging HttpRequestContext for the time being and changing this later if it saves us significant amounts of time. Just not sure if changing this in the future is going to be a headache. |
Just looked at https://github.com/elastic/kibana/pull/61797/files, and I was wondering if (yet another) layer of generics couldn't solve this issue atm export type RequestHandler<
P = unknown,
Q = unknown,
B = unknown,
Method extends RouteMethod = any
> = (
context: RequestHandlerContext,
request: KibanaRequest<P, Q, B, Method>,
response: KibanaResponseFactory
) => IKibanaResponse<any> | Promise<IKibanaResponse<any>>;addition in PR export type HttpResourcesRequestHandler<P = unknown, Q = unknown, B = unknown> = (
context: RequestHandlerContext,
request: KibanaRequest<P, Q, B, 'get'>,
response: KibanaResponseFactory & HttpResourcesServiceToolkit
) => IKibanaResponse<any> | Promise<IKibanaResponse<any>>;what about export type RequestHandler<
P = unknown,
Q = unknown,
B = unknown,
Method extends RouteMethod = any,
ResponseFactory extends KibanaResponseFactory = KibanaResponseFactory,
> = (
context: RequestHandlerContext,
request: KibanaRequest<P, Q, B, Method>,
response: ResponseFactory
) => IKibanaResponse<any> | Promise<IKibanaResponse<any>>;and export interface HttpResources {
registerCoreApp: [...],
register: <P, Q, B>(
route: RouteConfig<P, Q, B, 'get'>,
handler: RequestHandler<P, Q, B, 'get', HttpResourcesResponseFactory>
) => void;
}or even type HttpResourcesRequestHandler<P = unknown, Q = unknown, B = unknown> = RequestHandler<P, Q, B, 'get', HttpResourcesResponseFactory>;
export interface HttpResources {
registerCoreApp: [...],
register: <P, Q, B>(
route: RouteConfig<P, Q, B, 'get'>,
handler: HttpResourcesRequestHandler<P, Q, B) => void;
}with
|
yeah, I don't want to overcomplicate the generics. However, it's not such a big problem if we provide wrapper type for a route handler from the core. Thus, plugins don't have to implement it manually |
|
The last piece of the task is to serve a dynamic asset from a route handler. that's used in the canvas plugin The canvas plugin has a workaround in place. So we can switch to more urgent migration tasks meantime. In the future, we need to figure out how to solve the problem without using |
|
This has been sitting without activity for quite some time now, and all tasks have been addressed other than dynamic assets. As mentioned in #50654 (comment), canvas is working around this, so I'm going to close this for now. We can open a new issue in the future if more use cases for dynamic assets come up. |
Discussion in #44620
HttpResourceService
There are 2 different cases for static resources:
/my-page/.Plan
The text was updated successfully, but these errors were encountered: