Skip to content

Commit

Permalink
[Data plugin] Wrong caching for Index pattern fields (#87116)
Browse files Browse the repository at this point in the history 
* [Data plugin] Wrong caching for Index pattern fields

Closes: #84666

* remove can update index_pattern fields test

* fix tests

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
@alexwizp @kibanamachine
alexwizp and kibanamachine authored Jan 5, 2021
1 parent 76b8c49 commit 7ed089c
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 202 deletions.
20 changes: 12 additions & 8 deletions src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import { IndexPatternMissingIndices } from '../lib';
import { findByTitle } from '../utils';
import { DuplicateIndexPatternError } from '../errors';

const indexPatternCache = createIndexPatternCache();
const MAX_ATTEMPTS_TO_RESOLVE_CONFLICTS = 3;
const savedObjectType = 'index-pattern';

Expand All @@ -72,6 +71,8 @@ export class IndexPatternsService {
private fieldFormats: FieldFormatsStartCommon;
private onNotification: OnNotification;
private onError: OnError;
private indexPatternCache: ReturnType<typeof createIndexPatternCache>;

ensureDefaultIndexPattern: EnsureDefaultIndexPattern;

constructor({
Expand All @@ -93,6 +94,8 @@ export class IndexPatternsService {
uiSettings,
onRedirectNoIndexPattern
);

this.indexPatternCache = createIndexPatternCache();
}

/**
Expand Down Expand Up @@ -175,9 +178,9 @@ export class IndexPatternsService {
clearCache = (id?: string) => {
this.savedObjectsCache = null;
if (id) {
indexPatternCache.clear(id);
this.indexPatternCache.clear(id);
} else {
indexPatternCache.clearAll();
this.indexPatternCache.clearAll();
}
};

Expand Down Expand Up @@ -435,11 +438,12 @@ export class IndexPatternsService {

get = async (id: string): Promise<IndexPattern> => {
const indexPatternPromise =
indexPatternCache.get(id) || indexPatternCache.set(id, this.getSavedObjectAndInit(id));
this.indexPatternCache.get(id) ||
this.indexPatternCache.set(id, this.getSavedObjectAndInit(id));

// don't cache failed requests
indexPatternPromise.catch(() => {
indexPatternCache.clear(id);
this.indexPatternCache.clear(id);
});

return indexPatternPromise;
Expand Down Expand Up @@ -503,7 +507,7 @@ export class IndexPatternsService {
id: indexPattern.id,
});
indexPattern.id = response.id;
indexPatternCache.set(indexPattern.id, Promise.resolve(indexPattern));
this.indexPatternCache.set(indexPattern.id, Promise.resolve(indexPattern));
return indexPattern;
}

Expand Down Expand Up @@ -586,7 +590,7 @@ export class IndexPatternsService {
indexPattern.version = samePattern.version;

// Clear cache
indexPatternCache.clear(indexPattern.id!);
this.indexPatternCache.clear(indexPattern.id!);

// Try the save again
return this.updateSavedObject(indexPattern, saveAttempts, ignoreErrors);
Expand All @@ -600,7 +604,7 @@ export class IndexPatternsService {
* @param indexPatternId: Id of kibana Index Pattern to delete
*/
async delete(indexPatternId: string) {
indexPatternCache.clear(indexPatternId);
this.indexPatternCache.clear(indexPatternId);
return this.savedObjectsClient.delete('index-pattern', indexPatternId);
}
}
Expand Down
135 changes: 34 additions & 101 deletions test/api_integration/apis/index_patterns/fields_api/update_fields/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,32 @@ import { FtrProviderContext } from '../../../../ftr_provider_context';

export default function ({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');

describe('main', () => {
const basicIndex = 'ba*ic_index';
let indexPattern: any;

before(async () => {
await esArchiver.load('index_patterns/basic_index');

indexPattern = (
await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title: basicIndex,
},
})
).body.index_pattern;
});

after(async () => {
await esArchiver.unload('index_patterns/basic_index');

if (indexPattern) {
await supertest.delete('/api/index_patterns/index_pattern/' + indexPattern.id);
}
});

it('can update multiple fields', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
Expand Down Expand Up @@ -171,48 +195,6 @@ export default function ({ getService }: FtrProviderContext) {
expect(response3.status).to.be(200);
expect(response3.body.index_pattern.fieldAttrs.foo.count).to.be(undefined);
});

it('can set field "count" attribute on an existing field', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title,
fields: {
foo: {
name: 'foo',
type: 'string',
count: 123,
},
},
},
});

expect(response1.status).to.be(200);
expect(response1.body.index_pattern.fieldAttrs.foo).to.be(undefined);
expect(response1.body.index_pattern.fields.foo.count).to.be(123);

const response2 = await supertest
.post(`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}/fields`)
.send({
fields: {
foo: {
count: 456,
},
},
});

expect(response2.status).to.be(200);
expect(response2.body.index_pattern.fieldAttrs.foo).to.be(undefined);
expect(response2.body.index_pattern.fields.foo.count).to.be(456);

const response3 = await supertest.get(
`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}`
);

expect(response3.status).to.be(200);
expect(response3.body.index_pattern.fieldAttrs.foo).to.be(undefined);
expect(response3.body.index_pattern.fields.foo.count).to.be(456);
});
});

describe('customLabel', () => {
Expand Down Expand Up @@ -323,46 +305,20 @@ export default function ({ getService }: FtrProviderContext) {
});

it('can set field "customLabel" attribute on an existing field', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title,
fields: {
foo: {
name: 'foo',
type: 'string',
count: 123,
customLabel: 'foo',
},
await supertest.post(`/api/index_patterns/index_pattern/${indexPattern.id}/fields`).send({
fields: {
foo: {
customLabel: 'baz',
},
},
});

expect(response1.status).to.be(200);
expect(response1.body.index_pattern.fieldAttrs.foo).to.be(undefined);
expect(response1.body.index_pattern.fields.foo.customLabel).to.be('foo');

const response2 = await supertest
.post(`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}/fields`)
.send({
fields: {
foo: {
customLabel: 'baz',
},
},
});

expect(response2.status).to.be(200);
expect(response2.body.index_pattern.fieldAttrs.foo).to.be(undefined);
expect(response2.body.index_pattern.fields.foo.customLabel).to.be('baz');

const response3 = await supertest.get(
`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}`
const response1 = await supertest.get(
`/api/index_patterns/index_pattern/${indexPattern.id}`
);

expect(response3.status).to.be(200);
expect(response3.body.index_pattern.fieldAttrs.foo).to.be(undefined);
expect(response3.body.index_pattern.fields.foo.customLabel).to.be('baz');
expect(response1.status).to.be(200);
expect(response1.body.index_pattern.fields.foo.customLabel).to.be('baz');
});
});

Expand Down Expand Up @@ -463,31 +419,8 @@ export default function ({ getService }: FtrProviderContext) {
});

it('can remove "format" attribute from index_pattern format map', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title,
fieldFormats: {
foo: {
id: 'bar',
params: {
baz: 'qux',
},
},
},
},
});

expect(response1.status).to.be(200);
expect(response1.body.index_pattern.fieldFormats.foo).to.eql({
id: 'bar',
params: {
baz: 'qux',
},
});

const response2 = await supertest
.post(`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}/fields`)
.post(`/api/index_patterns/index_pattern/${indexPattern.id}/fields`)
.send({
fields: {
foo: {
Expand All @@ -500,7 +433,7 @@ export default function ({ getService }: FtrProviderContext) {
expect(response2.body.index_pattern.fieldFormats.foo).to.be(undefined);

const response3 = await supertest.get(
`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}`
`/api/index_patterns/index_pattern/${indexPattern.id}`
);

expect(response3.status).to.be(200);
Expand Down
38 changes: 0 additions & 38 deletions test/api_integration/apis/index_patterns/index_pattern_crud/update_index_pattern/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,44 +262,6 @@ export default function ({ getService }: FtrProviderContext) {
expect(response3.body.index_pattern.typeMeta).to.eql({ foo: 'baz' });
});

it('can update index_pattern fields', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title,
fields: {
foo: {
name: 'foo',
type: 'string',
},
},
},
});

expect(response1.body.index_pattern.fields.foo.name).to.be('foo');
expect(response1.body.index_pattern.fields.foo.type).to.be('string');

const id = response1.body.index_pattern.id;
const response2 = await supertest.post('/api/index_patterns/index_pattern/' + id).send({
index_pattern: {
fields: {
bar: {
name: 'bar',
type: 'number',
},
},
},
});

expect(response2.body.index_pattern.fields.bar.name).to.be('bar');
expect(response2.body.index_pattern.fields.bar.type).to.be('number');

const response3 = await supertest.get('/api/index_patterns/index_pattern/' + id);

expect(response3.body.index_pattern.fields.bar.name).to.be('bar');
expect(response3.body.index_pattern.fields.bar.type).to.be('number');
});

it('can update multiple index pattern fields at once', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
Expand Down
51 changes: 28 additions & 23 deletions .../api_integration/apis/index_patterns/scripted_fields_crud/delete_scripted_field/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,33 @@ import expect from '@kbn/expect';
import { FtrProviderContext } from '../../../../ftr_provider_context';

export default function ({ getService }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const supertest = getService('supertest');

describe('errors', () => {
const basicIndex = 'b*sic_index';
let indexPattern: any;

before(async () => {
await esArchiver.load('index_patterns/basic_index');

indexPattern = (
await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title: basicIndex,
},
})
).body.index_pattern;
});

after(async () => {
await esArchiver.unload('index_patterns/basic_index');

if (indexPattern) {
await supertest.delete('/api/index_patterns/index_pattern/' + indexPattern.id);
}
});

it('returns 404 error on non-existing index_pattern', async () => {
const id = `xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx-${Date.now()}`;
const response = await supertest.delete(
Expand All @@ -34,35 +58,16 @@ export default function ({ getService }: FtrProviderContext) {
});

it('returns 404 error on non-existing scripted field', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title,
},
});
const response2 = await supertest.delete(
`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}/scripted_field/foo`
const response1 = await supertest.delete(
`/api/index_patterns/index_pattern/${indexPattern.id}/scripted_field/test`
);

expect(response2.status).to.be(404);
expect(response1.status).to.be(404);
});

it('returns error when attempting to delete a field which is not a scripted field', async () => {
const title = `foo-${Date.now()}-${Math.random()}*`;
const response1 = await supertest.post('/api/index_patterns/index_pattern').send({
index_pattern: {
title,
fields: {
foo: {
scripted: false,
name: 'foo',
type: 'string',
},
},
},
});
const response2 = await supertest.delete(
`/api/index_patterns/index_pattern/${response1.body.index_pattern.id}/scripted_field/foo`
`/api/index_patterns/index_pattern/${indexPattern.id}/scripted_field/foo`
);

expect(response2.status).to.be(400);
Expand Down
Loading

0 comments on commit 7ed089c

Please sign in to comment.