[8.x] [ML] Data Frame Analytics: Fix field name escaping for Vega based scatterplot matrix. (#193386) (#193832)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] Data Frame Analytics: Fix field name escaping for Vega based
scatterplot matrix.
(#193386)](https://github.com/elastic/kibana/pull/193386)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Walter
Rafelsberger","email":"walter.rafelsberger@elastic.co"},"sourceCommit":{"committedDate":"2024-09-24T08:41:46Z","message":"[ML]
Data Frame Analytics: Fix field name escaping for Vega based scatterplot
matrix. (#193386)\n\n## Summary\r\n\r\nField names with `\\n` would fail
to render the DFA scatterplot matrix:\r\n\r\n<img width=\"804\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/26e356b8-236d-4255-b556-2ebc2e5db4fc\">\r\n\r\nThis
fixes the escaping and adds unit tests.\r\n\r\nThe fix isn't 100% ideal
because there are cases when we may end up with\r\nan additional
backslash being rendered for labels of the scatterplot.\r\nHowever, all
other variations I tried caused rendering problems of the\r\ncharts and
rendering would fail completely.\r\n\r\nFor example, just escaping `\\n`
without the general backslash escaping\r\ncauses the following Vega
error: `Duplicate scale or projection
name:\r\n\"child__row_my_numbercolumn_my_number_x\"`\r\n\r\nOn the other
hand escaping just the backslash without the additional\r\n`\\n`
escaping causes an \"expression parse error\" in in Vega and
the\r\nchart wouldn't render.\r\n\r\nNote this PR just focuses on
escaping for the Vega spec for the\r\nscatterplot matrix. There are
still other places in the UI (data grid\r\nheaders, fields
selector).\r\n\r\n<img width=\"792\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/35532741-7a13-4707-b8da-c72dcc8c935b\">\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] This was
checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"194d6307dc41b6a4a295abc3e412de148e05386e","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug",":ml","release_note:skip","Feature:Data
Frame
Analytics","v9.0.0","backport:all-open","v8.16.0","v8.15.2","v7.17.25"],"title":"[ML]
Data Frame Analytics: Fix field name escaping for Vega based scatterplot
matrix.","number":193386,"url":"https://github.com/elastic/kibana/pull/193386","mergeCommit":{"message":"[ML]
Data Frame Analytics: Fix field name escaping for Vega based scatterplot
matrix. (#193386)\n\n## Summary\r\n\r\nField names with `\\n` would fail
to render the DFA scatterplot matrix:\r\n\r\n<img width=\"804\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/26e356b8-236d-4255-b556-2ebc2e5db4fc\">\r\n\r\nThis
fixes the escaping and adds unit tests.\r\n\r\nThe fix isn't 100% ideal
because there are cases when we may end up with\r\nan additional
backslash being rendered for labels of the scatterplot.\r\nHowever, all
other variations I tried caused rendering problems of the\r\ncharts and
rendering would fail completely.\r\n\r\nFor example, just escaping `\\n`
without the general backslash escaping\r\ncauses the following Vega
error: `Duplicate scale or projection
name:\r\n\"child__row_my_numbercolumn_my_number_x\"`\r\n\r\nOn the other
hand escaping just the backslash without the additional\r\n`\\n`
escaping causes an \"expression parse error\" in in Vega and
the\r\nchart wouldn't render.\r\n\r\nNote this PR just focuses on
escaping for the Vega spec for the\r\nscatterplot matrix. There are
still other places in the UI (data grid\r\nheaders, fields
selector).\r\n\r\n<img width=\"792\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/35532741-7a13-4707-b8da-c72dcc8c935b\">\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] This was
checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"194d6307dc41b6a4a295abc3e412de148e05386e"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.15","7.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193386","number":193386,"mergeCommit":{"message":"[ML]
Data Frame Analytics: Fix field name escaping for Vega based scatterplot
matrix. (#193386)\n\n## Summary\r\n\r\nField names with `\\n` would fail
to render the DFA scatterplot matrix:\r\n\r\n<img width=\"804\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/26e356b8-236d-4255-b556-2ebc2e5db4fc\">\r\n\r\nThis
fixes the escaping and adds unit tests.\r\n\r\nThe fix isn't 100% ideal
because there are cases when we may end up with\r\nan additional
backslash being rendered for labels of the scatterplot.\r\nHowever, all
other variations I tried caused rendering problems of the\r\ncharts and
rendering would fail completely.\r\n\r\nFor example, just escaping `\\n`
without the general backslash escaping\r\ncauses the following Vega
error: `Duplicate scale or projection
name:\r\n\"child__row_my_numbercolumn_my_number_x\"`\r\n\r\nOn the other
hand escaping just the backslash without the additional\r\n`\\n`
escaping causes an \"expression parse error\" in in Vega and
the\r\nchart wouldn't render.\r\n\r\nNote this PR just focuses on
escaping for the Vega spec for the\r\nscatterplot matrix. There are
still other places in the UI (data grid\r\nheaders, fields
selector).\r\n\r\n<img width=\"792\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/35532741-7a13-4707-b8da-c72dcc8c935b\">\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] This was
checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"194d6307dc41b6a4a295abc3e412de148e05386e"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.15","label":"v8.15.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"7.17","label":"v7.17.25","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Walter Rafelsberger <walter.rafelsberger@elastic.co>
This commit is contained in:
Kibana Machine 2024-09-24 20:15:32 +10:00 committed by GitHub
parent ce7477005f
commit 5e064513bf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 71 additions and 3 deletions

View file

@ -16,6 +16,7 @@ import { LEGEND_TYPES } from '../vega_chart/common';
import {
getColorSpec,
getEscapedVegaFieldName,
getScatterplotMatrixVegaLiteSpec,
COLOR_RANGE_NOMINAL,
COLOR_RANGE_OUTLIER,
@ -75,6 +76,56 @@ describe('getColorSpec()', () => {
});
});
describe('getEscapedVegaFieldName()', () => {
it('should escape dots in field names', () => {
const fieldName = 'field.name';
const escapedFieldName = getEscapedVegaFieldName(fieldName);
expect(escapedFieldName).toBe('field\\.name');
});
it('should escape brackets in field names', () => {
const fieldName = 'field[name]';
const escapedFieldName = getEscapedVegaFieldName(fieldName);
expect(escapedFieldName).toBe('field\\[name\\]');
});
it('should escape both dots and brackets in field names', () => {
const fieldName = 'field.name[0]';
const escapedFieldName = getEscapedVegaFieldName(fieldName);
expect(escapedFieldName).toBe('field\\.name\\[0\\]');
});
it('should return the same string if there are no special characters', () => {
const fieldName = 'fieldname';
const escapedFieldName = getEscapedVegaFieldName(fieldName);
expect(escapedFieldName).toBe('fieldname');
});
it('should prepend a string if provided', () => {
const fieldName = 'field.name';
const prependString = 'prefix_';
const escapedFieldName = getEscapedVegaFieldName(fieldName, prependString);
expect(escapedFieldName).toBe('prefix_field\\.name');
});
it('should escape newlines in field names', () => {
// String quotes process backslashes, so we need to escape them for
// the test string to contain a backslash. For example, without the
// double backslash, this string would contain a newline character.
const fieldName = 'field\\name';
const escapedFieldName = getEscapedVegaFieldName(fieldName);
expect(escapedFieldName).toBe('field\\\\name');
});
it('should escape backslashes in field names', () => {
// String quotes process backslashes, so we need to escape them for
// the test string to contain a backslash.
const fieldName = 'fieldname\\withbackslash';
const escapedFieldName = getEscapedVegaFieldName(fieldName);
expect(escapedFieldName).toBe('fieldname\\\\withbackslash');
});
});
describe('getScatterplotMatrixVegaLiteSpec()', () => {
const forCustomLink = false;

View file

@ -249,11 +249,28 @@ const getVegaSpecLayer = (
};
};
// Escapes the characters .[] in field names with double backslashes
// Escapes the characters .[]\ in field names with double backslashes
// since VEGA treats dots/brackets in field names as nested values.
// See https://vega.github.io/vega-lite/docs/field.html for details.
function getEscapedVegaFieldName(fieldName: string, prependString: string = '') {
return `${prependString}${fieldName.replace(/([\.|\[|\]])/g, '\\$1')}`;
export function getEscapedVegaFieldName(fieldName: string, prependString: string = '') {
// Note the following isn't 100% ideal because there are cases when we may
// end up with an additional backslash being rendered for labels of the
// scatterplot. However, all other variations I tried caused rendering
// problems of the charts and rendering would fail completely.
// For example, just escaping \n in the first replace without the general
// backslash escaping causes the following Vega error:
// Duplicate scale or projection name: "child__row_my_numbercolumn_my_number_x"
// Escaping just the backslash without the additional \n escaping causes
// causes an "expression parse error" in Vega and the chart wouldn't render.
// Escape newline characters
fieldName = fieldName.replace(/\n/g, '\\n');
// Escape .[]\
fieldName = fieldName.replace(/([\.|\[|\]|\\])/g, '\\$1');
return `${prependString}${fieldName}`;
}
type VegaValue = Record<string, string | number>;