Skip to content

Commit

Permalink
[Management] Enable index pattern version conflicts (#18937) (#19300)
Browse files Browse the repository at this point in the history
* Pass the version along so we can get version conflict errors, then try and resolve if we can

* Update the version post save

* Refactor slightly to match setId pattern

* Tests and updates to ensure the actual changes aren't clobbered

* Ensure we return the id

* Avoid infinite recursion, welcome suggestions on how to unit test this

* Change logic for refresh fields UI button. Now it will re-call init and force a fields refresh. This ensures we pick up on field format (and other) changes

* Fix a couple issues with saving field formats, #19037

* Use the right key for version
  • Loading branch information
chrisronline authored May 22, 2018
1 parent dc16506 commit 55b8130
Show file tree
Hide file tree
Showing 4 changed files with 248 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ uiModules.get('apps/management')
const confirmModalOptions = {
confirmButtonText: 'Refresh',
onConfirm: async () => {
await $scope.indexPattern.refreshFields();
await $scope.indexPattern.init(true);
$scope.fields = $scope.indexPattern.getNonScriptedFields();
},
title: 'Refresh field list?'
Expand Down
12 changes: 10 additions & 2 deletions src/ui/public/field_editor/field_editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ uiModules
}

if (!self.selectedFormatId) {
delete indexPattern.fieldFormatMap[field.name];
indexPattern.fieldFormatMap[field.name] = {};
} else {
indexPattern.fieldFormatMap[field.name] = self.field.format;
}
Expand Down Expand Up @@ -116,12 +116,20 @@ uiModules
const changedFormat = cur !== prev;
const missingFormat = cur && (!format || format.type.id !== cur);

if (!changedFormat || !missingFormat) return;
if (!changedFormat || !missingFormat) {
return;
}

// reset to the defaults, but make sure it's an object
const FieldFormat = getFieldFormatType();
const paramDefaults = new FieldFormat({}, getConfig).getParamDefaults();
const currentFormatParams = self.formatParams;
self.formatParams = _.assign({}, _.cloneDeep(paramDefaults));
// If there are no current or new params, the watch will not trigger
// so manually update the format here
if (_.size(currentFormatParams) === 0 && _.size(self.formatParams) === 0) {
self.field.format = new FieldFormat(self.formatParams, getConfig);
}
});

$scope.$watch('editor.formatParams', function () {
Expand Down
155 changes: 155 additions & 0 deletions src/ui/public/index_patterns/__tests__/_index_pattern.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import { IndexPatternProvider } from '../_index_pattern';

jest.mock('../../errors', () => ({
SavedObjectNotFound: jest.fn(),
DuplicateField: jest.fn(),
IndexPatternMissingIndices: jest.fn(),
}));

jest.mock('../../registry/field_formats', () => ({
fieldFormats: {
getDefaultInstance: jest.fn(),
}
}));

jest.mock('../../utils/mapping_setup', () => ({
expandShorthand: jest.fn().mockImplementation(() => ({
id: true,
title: true,
}))
}));

jest.mock('../../notify', () => ({
Notifier: jest.fn().mockImplementation(() => ({
error: jest.fn(),
})),
toastNotifications: {
addDanger: jest.fn(),
}
}));

jest.mock('../_format_hit', () => ({
formatHit: jest.fn().mockImplementation(() => ({
formatField: jest.fn(),
}))
}));

jest.mock('../_get', () => ({
IndexPatternsGetProvider: jest.fn().mockImplementation(() => ({
clearCache: jest.fn(),
}))
}));

jest.mock('../_intervals', () => ({
IndexPatternsIntervalsProvider: jest.fn(),
}));

jest.mock('../_field_list', () => ({
IndexPatternsFieldListProvider: jest.fn().mockImplementation((pattern) => {
return {
byName: {
id: { value: pattern.id },
title: { value: pattern.title },
},
every: jest.fn(),
};
})
}));

jest.mock('../_flatten_hit', () => ({
IndexPatternsFlattenHitProvider: jest.fn(),
}));

jest.mock('../_pattern_cache', () => ({
IndexPatternsPatternCacheProvider: {
clear: jest.fn(),
}
}));

jest.mock('../fields_fetcher_provider', () => ({
FieldsFetcherProvider: {
fetch: jest.fn().mockImplementation(() => ([]))
}
}));

jest.mock('../unsupported_time_patterns', () => ({
IsUserAwareOfUnsupportedTimePatternProvider: jest.fn(),
}));


jest.mock('../../saved_objects', () => {
const object = {
_version: 1,
_id: 'foo',
attributes: {
title: 'something'
}
};

return {
SavedObjectsClientProvider: {
get: async () => object,
update: async (type, id, body, { version }) => {
if (object._version !== version) {
throw {
statusCode: 409
};
}

object.attributes.title = body.title;

return {
id: object._id,
_version: ++object._version,
};
}
},
findObjectByTitle: jest.fn(),
};
});

const Private = arg => arg;
const config = {
get: jest.fn(),
watchAll: jest.fn(),
};
const Promise = window.Promise;
const confirmModalPromise = jest.fn();
const kbnUrl = {
eval: jest.fn(),
};

describe('IndexPattern', () => {
it('should handle version conflicts', async () => {
const IndexPattern = IndexPatternProvider(Private, config, Promise, confirmModalPromise, kbnUrl); // eslint-disable-line new-cap

// Create a normal index pattern
const pattern = new IndexPattern('foo');
await pattern.init();

expect(pattern.version).toBe(2);

// Create the same one - we're going to handle concurrency
const samePattern = new IndexPattern('foo');
await samePattern.init();

expect(samePattern.version).toBe(3);

// This will conflict because samePattern did a save (from refreshFields)
// but the resave should work fine
pattern.title = 'foo2';
await pattern.save();

// This should not be able to recover
samePattern.title = 'foo3';

let result;
try {
await samePattern.save();
} catch (err) {
result = err;
}

expect(result.statusCode).toBe(409);
});
});
92 changes: 82 additions & 10 deletions src/ui/public/index_patterns/_index_pattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { SavedObjectNotFound, DuplicateField, IndexPatternMissingIndices } from
import angular from 'angular';
import { fieldFormats } from '../registry/field_formats';
import UtilsMappingSetupProvider from '../utils/mapping_setup';
import { Notifier } from '../notify';
import { Notifier, toastNotifications } from '../notify';

import { getComputedFields } from './_get_computed_fields';
import { formatHit } from './_format_hit';
Expand All @@ -26,6 +26,8 @@ export function getRoutes() {
};
}

const MAX_ATTEMPTS_TO_RESOLVE_CONFLICTS = 3;

export function IndexPatternProvider(Private, config, Promise, confirmModalPromise, kbnUrl) {
const getConfig = (...args) => config.get(...args);
const getIds = Private(IndexPatternsGetProvider)('id');
Expand Down Expand Up @@ -72,7 +74,7 @@ export function IndexPatternProvider(Private, config, Promise, confirmModalPromi
return FieldFormat && new FieldFormat(mapping.params, getConfig);
}

function updateFromElasticSearch(indexPattern, response) {
function updateFromElasticSearch(indexPattern, response, forceFieldRefresh = false) {
if (!response.found) {
const markdownSaveId = indexPattern.id.replace('*', '%2A');

Expand Down Expand Up @@ -109,7 +111,7 @@ export function IndexPatternProvider(Private, config, Promise, confirmModalPromi
}
}

return indexFields(indexPattern);
return indexFields(indexPattern, forceFieldRefresh);
}

function isFieldRefreshRequired(indexPattern) {
Expand All @@ -128,14 +130,14 @@ export function IndexPatternProvider(Private, config, Promise, confirmModalPromi
});
}

function indexFields(indexPattern) {
function indexFields(indexPattern, forceFieldRefresh = false) {
let promise = Promise.resolve();

if (!indexPattern.id) {
return promise;
}

if (isFieldRefreshRequired(indexPattern)) {
if (forceFieldRefresh || isFieldRefreshRequired(indexPattern)) {
promise = indexPattern.refreshFields();
}

Expand All @@ -149,6 +151,11 @@ export function IndexPatternProvider(Private, config, Promise, confirmModalPromi
return id;
}

function setVersion(indexPattern, version) {
indexPattern.version = version;
return version;
}

function watch(indexPattern) {
if (configWatchers.has(indexPattern)) {
return;
Expand Down Expand Up @@ -200,7 +207,7 @@ export function IndexPatternProvider(Private, config, Promise, confirmModalPromi
return getRoutes();
}

init() {
init(forceFieldRefresh = false) {
watch(this);

if (!this.id) {
Expand All @@ -211,14 +218,26 @@ export function IndexPatternProvider(Private, config, Promise, confirmModalPromi
.then(resp => {
// temporary compatability for savedObjectsClient

setVersion(this, resp._version);

return {
_id: resp.id,
_type: resp.type,
_source: _.cloneDeep(resp.attributes),
found: resp._version ? true : false
};
})
.then(response => updateFromElasticSearch(this, response))
// Do this before we attempt to update from ES
// since that call can potentially perform a save
.then(response => {
this.originalBody = this.prepBody();
return response;
})
.then(response => updateFromElasticSearch(this, response, forceFieldRefresh))
// Do it after to ensure we have the most up to date information
.then(() => {
this.originalBody = this.prepBody();
})
.then(() => this);
}

Expand Down Expand Up @@ -399,9 +418,62 @@ export function IndexPatternProvider(Private, config, Promise, confirmModalPromi
return await _create(potentialDuplicateByTitle.id);
}

save() {
return savedObjectsClient.update(type, this.id, this.prepBody())
.then(({ id }) => setId(this, id));
save(saveAttempts = 0) {
const body = this.prepBody();
// What keys changed since they last pulled the index pattern
const originalChangedKeys = Object.keys(body).filter(key => body[key] !== this.originalBody[key]);
return savedObjectsClient.update(type, this.id, body, { version: this.version })
.then(({ id, _version }) => {
setId(this, id);
setVersion(this, _version);
})
.catch(err => {
if (err.statusCode === 409 && saveAttempts++ < MAX_ATTEMPTS_TO_RESOLVE_CONFLICTS) {
const samePattern = new IndexPattern(this.id);
return samePattern.init()
.then(() => {
// What keys changed from now and what the server returned
const updatedBody = samePattern.prepBody();

// Build a list of changed keys from the server response
// and ensure we ignore the key if the server response
// is the same as the original response (since that is expected
// if we made a change in that key)
const serverChangedKeys = Object.keys(updatedBody).filter(key => {
return updatedBody[key] !== body[key] && this.originalBody[key] !== updatedBody[key];
});

let unresolvedCollision = false;
for (const originalKey of originalChangedKeys) {
for (const serverKey of serverChangedKeys) {
if (originalKey === serverKey) {
unresolvedCollision = true;
break;
}
}
}

if (unresolvedCollision) {
toastNotifications.addDanger('Unable to write index pattern! Refresh the page to get the most up to date changes for this index pattern.'); // eslint-disable-line max-len
throw err;
}

// Set the updated response on this object
serverChangedKeys.forEach(key => {
this[key] = samePattern[key];
});

setVersion(this, samePattern.version);

// Clear cache
patternCache.clear(this.id);

// Try the save again
return this.save(saveAttempts);
});
}
throw err;
});
}

refreshFields() {
Expand Down

0 comments on commit 55b8130

Please sign in to comment.