[kbnServer/extensions] formalize request factories with helper (#12697)

* [kbnServer/extensions] add server.addMemoizedFactoryToRequest() helper

* [server/uiSettings] use server.addMemoizedFactoryToRequest() helper

* [kbnServer/extensions] name `this` for clarity

src/server/kbn_server.js

@@ -21,6 +21,7 @@ import pluginsInitializeMixin from './plugins/initialize';
 import { indexPatternsMixin } from './index_patterns';
 import { savedObjectsMixin } from './saved_objects';
 import { statsMixin } from './stats';
+import { serverExtensionsMixin } from './server_extensions';
 
 const rootDir = fromRoot('.');
 
@@ -37,6 +38,8 @@ export default class KbnServer {
       configSetupMixin,
       // sets this.server
       httpMixin,
+      // adds methods for extending this.server
+      serverExtensionsMixin,
       loggingMixin,
       warningsMixin,
       statusMixin,

src/server/server_extensions/__tests__/add_memoized_factory_to_request.js

@@ -0,0 +1,101 @@
+import sinon from 'sinon';
+import expect from 'expect.js';
+
+import { serverExtensionsMixin } from '../server_extensions_mixin';
+
+describe('server.addMemoizedFactoryToRequest()', () => {
+  const setup = () => {
+    class Request {}
+
+    class Server {
+      constructor() {
+        sinon.spy(this, 'decorate');
+      }
+      decorate(type, name, value) {
+        switch (type) {
+          case 'request': return Request.prototype[name] = value;
+          case 'server': return Server.prototype[name] = value;
+          default: throw new Error(`Unexpected decorate type ${type}`);
+        }
+      }
+    }
+
+    const server = new Server();
+    serverExtensionsMixin({}, server);
+    return { server, Request };
+  };
+
+  it('throws when propertyName is not a string', () => {
+    const { server } = setup();
+    expect(() => server.addMemoizedFactoryToRequest()).to.throwError('propertyName must be a string');
+    expect(() => server.addMemoizedFactoryToRequest(null)).to.throwError('propertyName must be a string');
+    expect(() => server.addMemoizedFactoryToRequest(1)).to.throwError('propertyName must be a string');
+    expect(() => server.addMemoizedFactoryToRequest(true)).to.throwError('propertyName must be a string');
+    expect(() => server.addMemoizedFactoryToRequest(/abc/)).to.throwError('propertyName must be a string');
+    expect(() => server.addMemoizedFactoryToRequest(['foo'])).to.throwError('propertyName must be a string');
+    expect(() => server.addMemoizedFactoryToRequest([1])).to.throwError('propertyName must be a string');
+    expect(() => server.addMemoizedFactoryToRequest({})).to.throwError('propertyName must be a string');
+  });
+
+  it('throws when factory is not a function', () => {
+    const { server } = setup();
+    expect(() => server.addMemoizedFactoryToRequest('name')).to.throwError('factory must be a function');
+    expect(() => server.addMemoizedFactoryToRequest('name', null)).to.throwError('factory must be a function');
+    expect(() => server.addMemoizedFactoryToRequest('name', 1)).to.throwError('factory must be a function');
+    expect(() => server.addMemoizedFactoryToRequest('name', true)).to.throwError('factory must be a function');
+    expect(() => server.addMemoizedFactoryToRequest('name', /abc/)).to.throwError('factory must be a function');
+    expect(() => server.addMemoizedFactoryToRequest('name', ['foo'])).to.throwError('factory must be a function');
+    expect(() => server.addMemoizedFactoryToRequest('name', [1])).to.throwError('factory must be a function');
+    expect(() => server.addMemoizedFactoryToRequest('name', {})).to.throwError('factory must be a function');
+  });
+
+  it('throws when factory takes more than one arg', () => {
+    const { server } = setup();
+    /* eslint-disable no-unused-vars */
+    expect(() => server.addMemoizedFactoryToRequest('name', () => {})).to.not.throwError('more than one argument');
+    expect(() => server.addMemoizedFactoryToRequest('name', (a) => {})).to.not.throwError('more than one argument');
+
+    expect(() => server.addMemoizedFactoryToRequest('name', (a, b) => {})).to.throwError('more than one argument');
+    expect(() => server.addMemoizedFactoryToRequest('name', (a, b, c) => {})).to.throwError('more than one argument');
+    expect(() => server.addMemoizedFactoryToRequest('name', (a, b, c, d) => {})).to.throwError('more than one argument');
+    expect(() => server.addMemoizedFactoryToRequest('name', (a, b, c, d, e) => {})).to.throwError('more than one argument');
+    /* eslint-enable no-unused-vars */
+  });
+
+  it('decorates request objects with a function at `propertyName`', () => {
+    const { server, Request } = setup();
+
+    expect(new Request()).to.not.have.property('decorated');
+    server.addMemoizedFactoryToRequest('decorated', () => {});
+    expect(new Request()).to.have.property('decorated').a('function');
+  });
+
+  it('caches invocations of the factory to the request instance', () => {
+    const { server, Request } = setup();
+    const factory = sinon.stub().returnsArg(0);
+    server.addMemoizedFactoryToRequest('foo', factory);
+
+    const request1 = new Request();
+    const request2 = new Request();
+
+    // call `foo()` on both requests a bunch of times, each time
+    // the return value should be exactly the same
+    expect(request1.foo()).to.be(request1);
+    expect(request1.foo()).to.be(request1);
+    expect(request1.foo()).to.be(request1);
+    expect(request1.foo()).to.be(request1);
+    expect(request1.foo()).to.be(request1);
+    expect(request1.foo()).to.be(request1);
+
+    expect(request2.foo()).to.be(request2);
+    expect(request2.foo()).to.be(request2);
+    expect(request2.foo()).to.be(request2);
+    expect(request2.foo()).to.be(request2);
+
+    // only two different requests, so factory should have only been
+    // called twice with the two request objects
+    sinon.assert.calledTwice(factory);
+    sinon.assert.calledWithExactly(factory, request1);
+    sinon.assert.calledWithExactly(factory, request2);
+  });
+});

src/server/server_extensions/index.js

@@ -0,0 +1 @@
+export { serverExtensionsMixin } from './server_extensions_mixin';

src/server/server_extensions/server_extensions_mixin.js

@@ -0,0 +1,46 @@
+
+export function serverExtensionsMixin(kbnServer, server) {
+  /**
+   *  Decorate all request objects with a new method, `methodName`,
+   *  that will call the `factory` on first invocation and return
+   *  the result of the first call to subsequent invocations.
+   *
+   *  @method server.addMemoizedFactoryToRequest
+   *  @param {string} methodName location on the request this
+   *                             factory should be added
+   *  @param {Function} factory the factory to add to the request,
+   *                            which will be called once per request
+   *                            with a single argument, the request.
+   *  @return {undefined}
+   */
+  server.decorate('server', 'addMemoizedFactoryToRequest', (methodName, factory) => {
+    if (typeof methodName !== 'string') {
+      throw new TypeError('methodName must be a string');
+    }
+
+    if (typeof factory !== 'function') {
+      throw new TypeError('factory must be a function');
+    }
+
+    if (factory.length > 1) {
+      throw new TypeError(`
+        factory must not take more than one argument, the request object.
+        Memoization is done based on the request instance and is cached and reused
+        regardless of other arguments. If you want to have a per-request cache that
+        also does some sort of secondary memoization then return an object or function
+        from the memoized decorator and do secordary memoization there.
+      `);
+    }
+
+    const requestCache = new WeakMap();
+    server.decorate('request', methodName, function () {
+      const request = this;
+
+      if (!requestCache.has(request)) {
+        requestCache.set(request, factory(request));
+      }
+
+      return requestCache.get(request);
+    });
+  });
+}

src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js

@@ -38,6 +38,11 @@ describe('uiSettingsMixin()', () => {
     const server = {
       log: sinon.stub(),
       config: () => config,
+      addMemoizedFactoryToRequest(name, factory) {
+        this.decorate('request', name, function () {
+          return factory(this);
+        });
+      },
       decorate: sinon.spy((type, name, value) => {
         decorations[type][name] = value;
       }),

src/ui/ui_settings/ui_settings_mixin.js

@@ -44,8 +44,8 @@ export function uiSettingsMixin(kbnServer, server, config) {
     });
   });
 
-  server.decorate('request', 'getUiSettingsService', function () {
-    return getUiSettingsServiceForRequest(server, this, {
+  server.addMemoizedFactoryToRequest('getUiSettingsService', request => {
+    return getUiSettingsServiceForRequest(server, request, {
       getDefaults,
       readInterceptor,
     });

src/ui/ui_settings/ui_settings_service_for_request.js

@@ -1,7 +1,5 @@
 import { uiSettingsServiceFactory } from './ui_settings_service_factory';
 
-const BY_REQUEST_CACHE = new WeakMap();
-
 /**
  *  Get/create an instance of UiSettingsService bound to a specific request.
  *  Each call is cached (keyed on the request object itself) and subsequent
@@ -19,10 +17,6 @@ const BY_REQUEST_CACHE = new WeakMap();
  *  @return {UiSettingsService}
  */
 export function getUiSettingsServiceForRequest(server, request, options = {}) {
-  if (BY_REQUEST_CACHE.has(request)) {
-    return BY_REQUEST_CACHE.get(request);
-  }
-
   const {
     readInterceptor,
     getDefaults
@@ -34,6 +28,5 @@ export function getUiSettingsServiceForRequest(server, request, options = {}) {
     savedObjectsClient: request.getSavedObjectsClient()
   });
 
-  BY_REQUEST_CACHE.set(request, uiSettingsService);
   return uiSettingsService;
 }