Skip to content

Commit

Permalink
SavedObjectsRepository.incrementCounter supports array of fields
Browse files Browse the repository at this point in the history
  • Loading branch information
rudolf committed Nov 25, 2020
1 parent 4e9afee commit 7cb73eb
Show file tree
Hide file tree
Showing 15 changed files with 237 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

## SavedObjectsRepository.incrementCounter() method

Increases a counter field by one. Creates the document if one doesn't exist for the given id.
Increments all the specified counter fields by one. Creates the document if one doesn't exist for the given id.

<b>Signature:</b>

```typescript
incrementCounter(type: string, id: string, counterFieldName: string, options?: SavedObjectsIncrementCounterOptions): Promise<SavedObject>;
incrementCounter(type: string, id: string, counterFieldNames: string[], options?: SavedObjectsIncrementCounterOptions): Promise<SavedObject>;
```

## Parameters
Expand All @@ -18,7 +18,7 @@ incrementCounter(type: string, id: string, counterFieldName: string, options?: S
| --- | --- | --- |
| type | <code>string</code> | |
| id | <code>string</code> | |
| counterFieldName | <code>string</code> | |
| counterFieldNames | <code>string[]</code> | |
| options | <code>SavedObjectsIncrementCounterOptions</code> | |

<b>Returns:</b>
Expand All @@ -27,3 +27,27 @@ incrementCounter(type: string, id: string, counterFieldName: string, options?: S

{<!-- -->promise<!-- -->}

## Remarks

When using incrementCounter for collecting usage data, you need to ensure that usage collection happens on a best-effort basis and doesn't negatively affect your plugin or users (see the example): - Swallow any exceptions thrown from the incrementCounter method and log a message in development. - Don't block your application on the incrementCounter method (e.g. don't use `await`<!-- -->)

## Example

Collecting usage data

```ts
const repository = coreStart.savedObjects.createInternalRepository();

// NOTE: Usage collection happens on a best-effort basis, so we don't
// `await` the promise returned by `incrementCounter` and we swallow any
// exceptions in production.
repository
.incrementCounter('test_counter_type', 'counter_2', [
'stats.api.count',
'stats.api.count2',
'stats.total',
])
.catch((e) => (coreContext.env.cliArgs.dev ? logger.error(e) : e));

```

Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export declare class SavedObjectsRepository
| [deleteFromNamespaces(type, id, namespaces, options)](./kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md) | | Removes one or more namespaces from a given multi-namespace saved object. If no namespaces remain, the saved object is deleted entirely. This method and \[<code>addToNamespaces</code>\][SavedObjectsRepository.addToNamespaces()](./kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md) are the only ways to change which Spaces a multi-namespace saved object is shared to. |
| [find(options)](./kibana-plugin-core-server.savedobjectsrepository.find.md) | | |
| [get(type, id, options)](./kibana-plugin-core-server.savedobjectsrepository.get.md) | | Gets a single object |
| [incrementCounter(type, id, counterFieldName, options)](./kibana-plugin-core-server.savedobjectsrepository.incrementcounter.md) | | Increases a counter field by one. Creates the document if one doesn't exist for the given id. |
| [incrementCounter(type, id, counterFieldNames, options)](./kibana-plugin-core-server.savedobjectsrepository.incrementcounter.md) | | Increments all the specified counter fields by one. Creates the document if one doesn't exist for the given id. |
| [removeReferencesTo(type, id, options)](./kibana-plugin-core-server.savedobjectsrepository.removereferencesto.md) | | Updates all objects containing a reference to the given {<!-- -->type, id<!-- -->} tuple to remove the said reference. |
| [update(type, id, attributes, options)](./kibana-plugin-core-server.savedobjectsrepository.update.md) | | Updates an object |

Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* 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 { InternalCoreStart } from 'src/core/server/internal_types';
import * as kbnTestServer from '../../../../../test_helpers/kbn_server';
import { Root } from '../../../../root';

const { startES } = kbnTestServer.createTestServers({
adjustTimeout: (t: number) => jest.setTimeout(t),
});
let esServer: kbnTestServer.TestElasticsearchUtils;

describe('SavedObjectsRepository', () => {
let root: Root;
let start: InternalCoreStart;

beforeAll(async () => {
esServer = await startES();
root = kbnTestServer.createRootWithCorePlugins({
server: {
basePath: '/hello',
},
});

const setup = await root.setup();
setup.savedObjects.registerType({
hidden: false,
mappings: {
dynamic: false,
properties: {},
},
name: 'test_counter_type',
namespaceType: 'single',
});
start = await root.start();
});

afterAll(async () => {
await esServer.stop();
await root.shutdown();
});

describe('#incrementCounter', () => {
it('initializes a new document if none exists', async () => {
const now = new Date().getTime();
const repository = start.savedObjects.createInternalRepository();
await repository.incrementCounter('test_counter_type', 'counter_1', [
'stats.api.count',
'stats.api.count2',
'stats.total',
]);
const result = await repository.get('test_counter_type', 'counter_1');
expect(result.attributes).toMatchInlineSnapshot(`
Object {
"stats.api.count": 1,
"stats.api.count2": 1,
"stats.total": 1,
}
`);
expect(Date.parse(result.updated_at!)).toBeGreaterThanOrEqual(now);
});
it('increments the specified counters of an existing document', async () => {
const repository = start.savedObjects.createInternalRepository();
// Create document
await repository.incrementCounter('test_counter_type', 'counter_2', [
'stats.api.count',
'stats.api.count2',
'stats.total',
]);

const now = new Date().getTime();
// Increment counters
await repository.incrementCounter('test_counter_type', 'counter_2', [
'stats.api.count',
'stats.api.count2',
'stats.total',
]);
const result = await repository.get('test_counter_type', 'counter_2');
expect(result.attributes).toMatchInlineSnapshot(`
Object {
"stats.api.count": 2,
"stats.api.count2": 2,
"stats.total": 2,
}
`);
expect(Date.parse(result.updated_at!)).toBeGreaterThanOrEqual(now);
});
});
});
58 changes: 33 additions & 25 deletions src/core/server/saved_objects/service/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3272,11 +3272,11 @@ describe('SavedObjectsRepository', () => {
describe('#incrementCounter', () => {
const type = 'config';
const id = 'one';
const field = 'buildNum';
const counterFields = ['buildNum', 'apiCallsCount'];
const namespace = 'foo-namespace';
const originId = 'some-origin-id';

const incrementCounterSuccess = async (type, id, field, options) => {
const incrementCounterSuccess = async (type, id, fields, options) => {
const isMultiNamespace = registry.isMultiNamespace(type);
if (isMultiNamespace) {
const response = getMockGetResponse({ type, id }, options?.namespace);
Expand All @@ -3295,33 +3295,36 @@ describe('SavedObjectsRepository', () => {
type,
...mockTimestampFields,
[type]: {
[field]: 8468,
...fields.reduce((acc, field) => {
acc[field] = 8468;
return acc;
}, {}),
defaultIndex: 'logstash-*',
},
},
},
})
);

const result = await savedObjectsRepository.incrementCounter(type, id, field, options);
const result = await savedObjectsRepository.incrementCounter(type, id, fields, options);
expect(client.get).toHaveBeenCalledTimes(isMultiNamespace ? 1 : 0);
return result;
};

describe('client calls', () => {
it(`should use the ES update action if type is not multi-namespace`, async () => {
await incrementCounterSuccess(type, id, field, { namespace });
await incrementCounterSuccess(type, id, counterFields, { namespace });
expect(client.update).toHaveBeenCalledTimes(1);
});

it(`should use the ES get action then update action if type is multi-namespace, ID is defined, and overwrite=true`, async () => {
await incrementCounterSuccess(MULTI_NAMESPACE_TYPE, id, field, { namespace });
await incrementCounterSuccess(MULTI_NAMESPACE_TYPE, id, counterFields, { namespace });
expect(client.get).toHaveBeenCalledTimes(1);
expect(client.update).toHaveBeenCalledTimes(1);
});

it(`defaults to a refresh setting of wait_for`, async () => {
await incrementCounterSuccess(type, id, field, { namespace });
await incrementCounterSuccess(type, id, counterFields, { namespace });
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({
refresh: 'wait_for',
Expand All @@ -3331,7 +3334,7 @@ describe('SavedObjectsRepository', () => {
});

it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => {
await incrementCounterSuccess(type, id, field, { namespace });
await incrementCounterSuccess(type, id, counterFields, { namespace });
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({
id: `${namespace}:${type}:${id}`,
Expand All @@ -3341,7 +3344,7 @@ describe('SavedObjectsRepository', () => {
});

it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => {
await incrementCounterSuccess(type, id, field);
await incrementCounterSuccess(type, id, counterFields);
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({
id: `${type}:${id}`,
Expand All @@ -3351,7 +3354,7 @@ describe('SavedObjectsRepository', () => {
});

it(`normalizes options.namespace from 'default' to undefined`, async () => {
await incrementCounterSuccess(type, id, field, { namespace: 'default' });
await incrementCounterSuccess(type, id, counterFields, { namespace: 'default' });
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({
id: `${type}:${id}`,
Expand All @@ -3361,7 +3364,7 @@ describe('SavedObjectsRepository', () => {
});

it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => {
await incrementCounterSuccess(NAMESPACE_AGNOSTIC_TYPE, id, field, { namespace });
await incrementCounterSuccess(NAMESPACE_AGNOSTIC_TYPE, id, counterFields, { namespace });
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({
id: `${NAMESPACE_AGNOSTIC_TYPE}:${id}`,
Expand All @@ -3370,7 +3373,7 @@ describe('SavedObjectsRepository', () => {
);

client.update.mockClear();
await incrementCounterSuccess(MULTI_NAMESPACE_TYPE, id, field, { namespace });
await incrementCounterSuccess(MULTI_NAMESPACE_TYPE, id, counterFields, { namespace });
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({
id: `${MULTI_NAMESPACE_TYPE}:${id}`,
Expand All @@ -3389,7 +3392,7 @@ describe('SavedObjectsRepository', () => {

it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.incrementCounter(type, id, field, {
savedObjectsRepository.incrementCounter(type, id, counterFields, {
namespace: ALL_NAMESPACES_STRING,
})
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
Expand All @@ -3398,7 +3401,7 @@ describe('SavedObjectsRepository', () => {
it(`throws when type is not a string`, async () => {
const test = async (type) => {
await expect(
savedObjectsRepository.incrementCounter(type, id, field)
savedObjectsRepository.incrementCounter(type, id, counterFields)
).rejects.toThrowError(`"type" argument must be a string`);
expect(client.update).not.toHaveBeenCalled();
};
Expand All @@ -3413,23 +3416,24 @@ describe('SavedObjectsRepository', () => {
const test = async (field) => {
await expect(
savedObjectsRepository.incrementCounter(type, id, field)
).rejects.toThrowError(`"counterFieldName" argument must be a string`);
).rejects.toThrowError(`"counterFieldNames" argument must be an array of strings`);
expect(client.update).not.toHaveBeenCalled();
};

await test(null);
await test(42);
await test(false);
await test({});
await test([null]);
await test([42]);
await test([false]);
await test([{}]);
await test([{}, false, 42, null, 'string']);
});

it(`throws when type is invalid`, async () => {
await expectUnsupportedTypeError('unknownType', id, field);
await expectUnsupportedTypeError('unknownType', id, counterFields);
expect(client.update).not.toHaveBeenCalled();
});

it(`throws when type is hidden`, async () => {
await expectUnsupportedTypeError(HIDDEN_TYPE, id, field);
await expectUnsupportedTypeError(HIDDEN_TYPE, id, counterFields);
expect(client.update).not.toHaveBeenCalled();
});

Expand All @@ -3439,7 +3443,9 @@ describe('SavedObjectsRepository', () => {
elasticsearchClientMock.createSuccessTransportRequestPromise(response)
);
await expect(
savedObjectsRepository.incrementCounter(MULTI_NAMESPACE_TYPE, id, field, { namespace })
savedObjectsRepository.incrementCounter(MULTI_NAMESPACE_TYPE, id, counterFields, {
namespace,
})
).rejects.toThrowError(createConflictError(MULTI_NAMESPACE_TYPE, id));
expect(client.get).toHaveBeenCalledTimes(1);
});
Expand All @@ -3452,8 +3458,8 @@ describe('SavedObjectsRepository', () => {

it(`migrates a document and serializes the migrated doc`, async () => {
const migrationVersion = mockMigrationVersion;
await incrementCounterSuccess(type, id, field, { migrationVersion });
const attributes = { buildNum: 1 }; // this is added by the incrementCounter function
await incrementCounterSuccess(type, id, counterFields, { migrationVersion });
const attributes = { buildNum: 1, apiCallsCount: 1 }; // this is added by the incrementCounter function
const doc = { type, id, attributes, migrationVersion, ...mockTimestampFields };
expectMigrationArgs(doc);

Expand All @@ -3476,6 +3482,7 @@ describe('SavedObjectsRepository', () => {
...mockTimestampFields,
config: {
buildNum: 8468,
apiCallsCount: 100,
defaultIndex: 'logstash-*',
},
originId,
Expand All @@ -3487,7 +3494,7 @@ describe('SavedObjectsRepository', () => {
const response = await savedObjectsRepository.incrementCounter(
'config',
'6.0.0-alpha1',
'buildNum',
['buildNum', 'apiCallsCount'],
{
namespace: 'foo-namespace',
}
Expand All @@ -3500,6 +3507,7 @@ describe('SavedObjectsRepository', () => {
version: mockVersion,
attributes: {
buildNum: 8468,
apiCallsCount: 100,
defaultIndex: 'logstash-*',
},
originId,
Expand Down
Loading

0 comments on commit 7cb73eb

Please sign in to comment.