Skip to content

Extend request handler with request scoped core capabilities#43103

Merged
joshdover merged 11 commits intoelastic:masterfrom
mshustov:issue-33783-request-context
Aug 19, 2019
Merged

Extend request handler with request scoped core capabilities#43103
joshdover merged 11 commits intoelastic:masterfrom
mshustov:issue-33783-request-context

Conversation

@mshustov
Copy link
Copy Markdown
Contributor

@mshustov mshustov commented Aug 12, 2019

Summary

Closes #33783

Introduces RequestHandlerContext passed to a request handler as the very first argument.

// my-plugin.ts
deps.http.registerRouteHandlerContext(
  'myApp',
  (context, req) => {
    async function search (id: string) {
      return await context.elasticsearch.adminClient.callAsInternalUser('endpoint', id);
    }
    return { search };
  }
);

// my-route-handler.ts
router.get({ path: '/', validate: false }, async (context, req, res) => {
  const response = await context.myApp.search(...);
  return res.ok(response);
});

Currently extended only with elasticsearch service, which provides cluster clients scoped to the current request.

declare module '../../server' {
  interface RequestHandlerContext {
    elasticsearch: {
      dataClient: ScopedClusterClient;
      adminClient: ScopedClusterClient;
    };
  }
}

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

Dev docs

Kibana facilitates new Handler context pattern to provide core services API to plugins. The central purpose of this pattern is to hide some implementation details and provide a set of API specific for the current context. For example, RequestHandlerContext contains tailored elasticsearch service API, letting a plugin to perform a request to elasticsearch on behalf of the user requesting Kibana. RequestHandlerContext is exposed to the route handlers as the very first argument.

class MyPlugin {
  setup(core) {
    const router = core.http.createRouter();
    router.get({ path: '/ping', validate: false }, async (context, req, res) => {
      const response = await context.core.elasticsearch.adminClient.callAsInternalUser('ping');
      return res.ok({ body: `Pong: ${response}` });
    });
  }
}

@mshustov mshustov added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.4.0 labels Aug 12, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-platform

@mshustov mshustov force-pushed the issue-33783-request-context branch from fe1f58a to 140bed8 Compare August 12, 2019 14:01
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests that plugin has access to the route handler context functionality provided by the core.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests that plugin has access to the route handler context functionality provided the dependency

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will have to change the signature if we want to use the same context declaration for route handlers and request interceptors.

request interceptors are out of the scope of the current PR.

@elastic elastic deleted a comment from elasticmachine Aug 12, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@mshustov mshustov marked this pull request as ready for review August 12, 2019 15:15
@mshustov mshustov requested a review from a team as a code owner August 12, 2019 15:15
import { Plugin, CoreSetup } from 'kibana/server';
import { PluginARequestContext } from '../../core_plugin_a/server';

declare module 'kibana/server' {
Copy link
Copy Markdown
Contributor Author

@mshustov mshustov Aug 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: this is necessary, as core_plugin_a is not a part of compilation setup

createRouter: (path: string) => {
const router = new Router(path, this.log);
createRouter: (path: string, pluginId: PluginOpaqueId = this.coreContext.coreId) => {
const enhanceHandler = this.requestHandlerContext!.createHandler.bind(null, pluginId);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the enhance name, maybe we should consider renaming IContextContainer.createHandler to IContextContainer.enhanceHandler

we have to import Context type from 'server' file to be able to reference extended type. otherwise, TS considers it empty
core.http.registerRouteHandlerContext('pluginA', context => {
return {
ping: () =>
context.core!.elasticsearch.adminClient.callAsInternalUser('ping') as Promise<string>,
Copy link
Copy Markdown
Contributor Author

@mshustov mshustov Aug 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO later: core context is always defined in plugin setup phase

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because of this type? https://github.com/elastic/kibana/blob/master/src/core/utils/context.ts#L42

I was a little conflicted on how this should work since plugins can also create ContextContainers, it's not clear that core will always be present. The only idea I have is adding an additional type parameter that defines which keyof TContext are always present.

@elastic elastic deleted a comment from elasticmachine Aug 13, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@mshustov mshustov requested a review from joshdover August 13, 2019 13:38
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@mshustov
Copy link
Copy Markdown
Contributor Author

retest

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[New platform] Extend request handler with request scoped core capabilities

4 participants