Enforce camelCase format for a plugin id

docs/development/core/public/kibana-plugin-public.savedobjectsclient.find.md

@@ -9,5 +9,5 @@ Search for objects
 <b>Signature:</b>
 
 ```typescript
-find: <T extends SavedObjectAttributes>(options: Pick<SavedObjectFindOptionsServer, "search" | "filter" | "type" | "page" | "fields" | "searchFields" | "defaultSearchOperator" | "hasReference" | "sortField" | "perPage">) => Promise<SavedObjectsFindResponsePublic<T>>;
+find: <T extends SavedObjectAttributes>(options: Pick<SavedObjectFindOptionsServer, "search" | "filter" | "type" | "page" | "perPage" | "sortField" | "fields" | "searchFields" | "hasReference" | "defaultSearchOperator">) => Promise<SavedObjectsFindResponsePublic<T>>;
 ```

docs/development/core/public/kibana-plugin-public.savedobjectsclient.md

@@ -24,7 +24,7 @@ The constructor for this class is marked as internal. Third-party code should no
 |  [bulkGet](./kibana-plugin-public.savedobjectsclient.bulkget.md) |  | <code>(objects?: {</code><br/><code>        id: string;</code><br/><code>        type: string;</code><br/><code>    }[]) =&gt; Promise&lt;SavedObjectsBatchResponse&lt;SavedObjectAttributes&gt;&gt;</code> | Returns an array of objects by id |
 |  [create](./kibana-plugin-public.savedobjectsclient.create.md) |  | <code>&lt;T extends SavedObjectAttributes&gt;(type: string, attributes: T, options?: SavedObjectsCreateOptions) =&gt; Promise&lt;SimpleSavedObject&lt;T&gt;&gt;</code> | Persists an object |
 |  [delete](./kibana-plugin-public.savedobjectsclient.delete.md) |  | <code>(type: string, id: string) =&gt; Promise&lt;{}&gt;</code> | Deletes an object |
-|  [find](./kibana-plugin-public.savedobjectsclient.find.md) |  | <code>&lt;T extends SavedObjectAttributes&gt;(options: Pick&lt;SavedObjectFindOptionsServer, &quot;search&quot; &#124; &quot;filter&quot; &#124; &quot;type&quot; &#124; &quot;page&quot; &#124; &quot;fields&quot; &#124; &quot;searchFields&quot; &#124; &quot;defaultSearchOperator&quot; &#124; &quot;hasReference&quot; &#124; &quot;sortField&quot; &#124; &quot;perPage&quot;&gt;) =&gt; Promise&lt;SavedObjectsFindResponsePublic&lt;T&gt;&gt;</code> | Search for objects |
+|  [find](./kibana-plugin-public.savedobjectsclient.find.md) |  | <code>&lt;T extends SavedObjectAttributes&gt;(options: Pick&lt;SavedObjectFindOptionsServer, &quot;search&quot; &#124; &quot;filter&quot; &#124; &quot;type&quot; &#124; &quot;page&quot; &#124; &quot;perPage&quot; &#124; &quot;sortField&quot; &#124; &quot;fields&quot; &#124; &quot;searchFields&quot; &#124; &quot;hasReference&quot; &#124; &quot;defaultSearchOperator&quot;&gt;) =&gt; Promise&lt;SavedObjectsFindResponsePublic&lt;T&gt;&gt;</code> | Search for objects |
 |  [get](./kibana-plugin-public.savedobjectsclient.get.md) |  | <code>&lt;T extends SavedObjectAttributes&gt;(type: string, id: string) =&gt; Promise&lt;SimpleSavedObject&lt;T&gt;&gt;</code> | Fetches a single object |
 
 ## Methods

docs/development/core/server/kibana-plugin-server.pluginmanifest.id.md

@@ -4,7 +4,7 @@
 
 ## PluginManifest.id property
 
-Identifier of the plugin.
+Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract. Other plugins leverage it to access plugin API, navigate to the plugin, etc.
 
 <b>Signature:</b>
 

docs/development/core/server/kibana-plugin-server.pluginmanifest.md

@@ -21,7 +21,7 @@ Should never be used in code outside of Core but is exported for documentation p
 |  Property | Type | Description |
 |  --- | --- | --- |
 |  [configPath](./kibana-plugin-server.pluginmanifest.configpath.md) | <code>ConfigPath</code> | Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id". |
-|  [id](./kibana-plugin-server.pluginmanifest.id.md) | <code>PluginName</code> | Identifier of the plugin. |
+|  [id](./kibana-plugin-server.pluginmanifest.id.md) | <code>PluginName</code> | Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract. Other plugins leverage it to access plugin API, navigate to the plugin, etc. |
 |  [kibanaVersion](./kibana-plugin-server.pluginmanifest.kibanaversion.md) | <code>string</code> | The version of Kibana the plugin is compatible with, defaults to "version". |
 |  [optionalPlugins](./kibana-plugin-server.pluginmanifest.optionalplugins.md) | <code>readonly PluginName[]</code> | An optional list of the other plugins that if installed and enabled \*\*may be\*\* leveraged by this plugin for some additional functionality but otherwise are not required for this plugin to work properly. |
 |  [requiredPlugins](./kibana-plugin-server.pluginmanifest.requiredplugins.md) | <code>readonly PluginName[]</code> | An optional list of the other plugins that \*\*must be\*\* installed and enabled for this plugin to function properly. |

src/core/CONVENTIONS.md

@@ -32,6 +32,7 @@ my_plugin/
     ├── index.ts
     └── plugin.ts
 ```
+- [Manifest file](/docs/development/core/server/kibana-plugin-server.pluginmanifest.md) should be defined on top level.
 - Both `server` and `public` should have an `index.ts` and a `plugin.ts` file:
   - `index.ts` should only contain:
     - The `plugin` export

src/core/server/config/integration_tests/config_deprecation.test.ts

@@ -36,7 +36,13 @@ describe('configuration deprecations', () => {
     await root.setup();
 
     const logs = loggingServiceMock.collect(mockLoggingService);
-    expect(logs.warn).toMatchInlineSnapshot(`Array []`);
+    const warnings = logs.warn.flatMap(i => i);
+    expect(warnings).not.toContain(
+      '"optimize.lazy" is deprecated and has been replaced by "optimize.watch"'
+    );
+    expect(warnings).not.toContain(
+      '"optimize.lazyPort" is deprecated and has been replaced by "optimize.watchPort"'
+    );
   });
 
   it('should log deprecation warnings for core deprecations', async () => {
@@ -50,15 +56,12 @@ describe('configuration deprecations', () => {
     await root.setup();
 
     const logs = loggingServiceMock.collect(mockLoggingService);
-    expect(logs.warn).toMatchInlineSnapshot(`
-      Array [
-        Array [
-          "\\"optimize.lazy\\" is deprecated and has been replaced by \\"optimize.watch\\"",
-        ],
-        Array [
-          "\\"optimize.lazyPort\\" is deprecated and has been replaced by \\"optimize.watchPort\\"",
-        ],
-      ]
-    `);
+    const warnings = logs.warn.flatMap(i => i);
+    expect(warnings).toContain(
+      '"optimize.lazy" is deprecated and has been replaced by "optimize.watch"'
+    );
+    expect(warnings).toContain(
+      '"optimize.lazyPort" is deprecated and has been replaced by "optimize.watchPort"'
+    );
   });
 });

src/core/server/plugins/discovery/is_camel_case.test.ts

@@ -0,0 +1,42 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { isCamelCase } from './is_camel_case';
+
+describe('isCamelCase', () => {
+  it('matches a string in camelCase', () => {
+    expect(isCamelCase('foo')).toBe(true);
+    expect(isCamelCase('foo1')).toBe(true);
+    expect(isCamelCase('fooBar')).toBe(true);
+    expect(isCamelCase('fooBarBaz')).toBe(true);
+    expect(isCamelCase('fooBAR')).toBe(true);
+  });
+
+  it('does not match strings in other cases', () => {
+    expect(isCamelCase('AAA')).toBe(false);
+    expect(isCamelCase('FooBar')).toBe(false);
+    expect(isCamelCase('3Foo')).toBe(false);
+    expect(isCamelCase('o_O')).toBe(false);
+    expect(isCamelCase('foo_bar')).toBe(false);
+    expect(isCamelCase('foo_')).toBe(false);
+    expect(isCamelCase('_fooBar')).toBe(false);
+    expect(isCamelCase('fooBar_')).toBe(false);
+    expect(isCamelCase('_fooBar_')).toBe(false);
+  });
+});

src/core/server/plugins/discovery/is_camel_case.ts

@@ -0,0 +1,22 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+const camelCaseRegExp = /^[a-z]{1}([a-zA-Z0-9]{1,})$/;
+export function isCamelCase(candidate: string) {
+  return camelCaseRegExp.test(candidate);
+}

src/core/server/plugins/discovery/plugin_manifest_parser.test.ts

@@ -20,10 +20,12 @@
 import { PluginDiscoveryErrorType } from './plugin_discovery_error';
 
 import { mockReadFile } from './plugin_manifest_parser.test.mocks';
+import { loggingServiceMock } from '../../logging/logging_service.mock';
 
 import { resolve } from 'path';
 import { parseManifest } from './plugin_manifest_parser';
 
+const logger = loggingServiceMock.createLogger();
 const pluginPath = resolve('path', 'existent-dir');
 const pluginManifestPath = resolve(pluginPath, 'kibana.json');
 const packageInfo = {
@@ -43,7 +45,7 @@ test('return error when manifest is empty', async () => {
     cb(null, Buffer.from(''));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Unexpected end of JSON input (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -55,7 +57,7 @@ test('return error when manifest content is null', async () => {
     cb(null, Buffer.from('null'));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Plugin manifest must contain a JSON encoded object. (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -67,7 +69,7 @@ test('return error when manifest content is not a valid JSON', async () => {
     cb(null, Buffer.from('not-json'));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Unexpected token o in JSON at position 1 (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -79,7 +81,7 @@ test('return error when plugin id is missing', async () => {
     cb(null, Buffer.from(JSON.stringify({ version: 'some-version' })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Plugin manifest must contain an "id" property. (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -91,19 +93,35 @@ test('return error when plugin id includes `.` characters', async () => {
     cb(null, Buffer.from(JSON.stringify({ id: 'some.name', version: 'some-version' })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Plugin "id" must not include \`.\` characters. (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
   });
 });
 
+test('logs warning if pluginId is not in camelCase format', async () => {
+  mockReadFile.mockImplementation((path, cb) => {
+    cb(null, Buffer.from(JSON.stringify({ id: 'some_name', version: 'kibana', server: true })));
+  });
+
+  expect(loggingServiceMock.collect(logger).warn).toHaveLength(0);
+  await parseManifest(pluginPath, packageInfo, logger);
+  expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(`
+    Array [
+      Array [
+        "Expect plugin \\"id\\" in camelCase, but found: some_name",
+      ],
+    ]
+  `);
+});
+
 test('return error when plugin version is missing', async () => {
   mockReadFile.mockImplementation((path, cb) => {
     cb(null, Buffer.from(JSON.stringify({ id: 'some-id' })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Plugin manifest for "some-id" must contain a "version" property. (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -115,7 +133,7 @@ test('return error when plugin expected Kibana version is lower than actual vers
     cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '6.4.2' })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Plugin "some-id" is only compatible with Kibana version "6.4.2", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.IncompatibleVersion,
     path: pluginManifestPath,
@@ -130,7 +148,7 @@ test('return error when plugin expected Kibana version cannot be interpreted as
     );
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Plugin "some-id" is only compatible with Kibana version "non-sem-ver", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.IncompatibleVersion,
     path: pluginManifestPath,
@@ -142,7 +160,7 @@ test('return error when plugin config path is not a string', async () => {
     cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', configPath: 2 })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `The "configPath" in plugin manifest for "some-id" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -157,7 +175,7 @@ test('return error when plugin config path is an array that contains non-string
     );
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `The "configPath" in plugin manifest for "some-id" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -169,7 +187,7 @@ test('return error when plugin expected Kibana version is higher than actual ver
     cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.1' })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Plugin "some-id" is only compatible with Kibana version "7.0.1", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.IncompatibleVersion,
     path: pluginManifestPath,
@@ -181,7 +199,7 @@ test('return error when both `server` and `ui` are set to `false` or missing', a
     cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0' })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "some-id", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -194,7 +212,7 @@ test('return error when both `server` and `ui` are set to `false` or missing', a
     );
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "some-id", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -217,7 +235,7 @@ test('return error when manifest contains unrecognized properties', async () =>
     );
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
     message: `Manifest for plugin "some-id" contains the following unrecognized properties: unknownOne,unknownTwo. (invalid-manifest, ${pluginManifestPath})`,
     type: PluginDiscoveryErrorType.InvalidManifest,
     path: pluginManifestPath,
@@ -229,7 +247,7 @@ test('set defaults for all missing optional fields', async () => {
     cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', server: true })));
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
     id: 'some-id',
     configPath: 'some-id',
     version: '7.0.0',
@@ -259,7 +277,7 @@ test('return all set optional fields as they are in manifest', async () => {
     );
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
     id: 'some-id',
     configPath: ['some', 'path'],
     version: 'some-version',
@@ -288,7 +306,7 @@ test('return manifest when plugin expected Kibana version matches actual version
     );
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
     id: 'some-id',
     configPath: 'some-path',
     version: 'some-version',
@@ -317,7 +335,7 @@ test('return manifest when plugin expected Kibana version is `kibana`', async ()
     );
   });
 
-  await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
+  await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
     id: 'some-id',
     configPath: 'some-id',
     version: 'some-version',