[Reporting] Define shims of legacy dependencies #54082
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,16 +4,22 @@ | |
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
|
|
||
| import { Legacy } from 'kibana'; | ||
| import { ServerFacade } from '../../types'; | ||
|
|
||
| export function getUserFactory(server: ServerFacade) { | ||
| /* | ||
| * Legacy.Request because this is called from routing middleware | ||
| */ | ||
| return async (request: Legacy.Request) => { | ||
|
There was a problem hiding this comment. This function takes a Legacy request because it is called from middleware. That means we get into this code before we get into any request handlers which is where we pick the properties for RequestFacade. There was a problem hiding this comment. @pgayvallet @restrry this means that I don't actually need to call There was a problem hiding this comment. ddf8fac added that method back, but for a different usage There was a problem hiding this comment. Just thinking loudly: why not call this function from a route handler? NP routing doesn't provide a pre-middleware mechanism at the moment. There was a problem hiding this comment.
Hm, if that is the case it will be a major blocker for Reporting migration. @rudolf can you confirm whether we will be able to register routing middleware in the new platform? There was a problem hiding this comment.
ATM this is not something we are planing to provide.
Correct me if I'm wrong, but the whole pre-routing mechanism can easily be replaced by some wrapper around the actual route handlers. The usage would be really close to what you currently have and should not be that big of a change Looking at your const getRouteConfig = () => {
const getOriginalRouteConfig: GetRouteConfigFactoryFn = getRouteConfigFactoryReportingPre(
server
);
const routeConfigFactory: RouteConfigFactory = getOriginalRouteConfig(
({ params: { exportType } }) => exportType
);
return {
...routeConfigFactory,
validate: {
params: Joi.object({
[...]
},
};
};
// generate report
server.route({
path: `${BASE_GENERATE}/{exportType}`,
method: 'POST',
options: getRouteConfig(),
handler: async (legacyRequest: Legacy.Request, h: ReportingResponseToolkit) => {
[...]
return response;
},
});would become something like (also adapted to NP router format) const getRouteConfig = () => {
const getOriginalRouteConfig: GetRouteConfigFactoryFn = getRouteConfigFactoryReportingPre(
server
);
const routeConfigFactory: RouteConfigFactory = getOriginalRouteConfig(
({ params: { exportType } }) => exportType
);
return {
...routeConfigFactory,
validate: {
params: schema.object({
[...]
},
};
};
const routeConfig = getRouteConfig()
router.post(
{
path: `${BASE_GENERATE}/{exportType}`,
options: routeConfig.options
validate: routeConfig.validate,
},
routeConfig.performPreAuth(async (ctx, req, res, preAuthResult) => {
const request = makeRequestFacade(legacyRequest, preAuthResults);
....
})
);The Please ping me if you want me to go a little more in-details with that. There was a problem hiding this comment. I'm ++ on any solution that will allow us to move further, and I don't see any problems with your proposals. I think there are a lot of ways to achieve the goal that the code is doing and pre-routing is just one of them. |
||
| if (!server.plugins.security) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| return await server.plugins.security.getUser(request); | ||
| } catch (err) { | ||
| server.log(['reporting', 'getUser', 'error'], err); | ||
|
|
||
| return null; | ||
| } | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved the
ts-ignoreto reporting/index.ts