[8.6] [Lens] gracefully handle JS Object method names as field names (#148118) (#149688)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Lens] gracefully handle JS Object method names as field names
(#148118)](https://github.com/elastic/kibana/pull/148118)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"marco.vettorello@elastic.co"},"sourceCommit":{"committedDate":"2023-01-27T13:40:51Z","message":"[Lens]
gracefully handle JS Object method names as field names (#148118)\n\nThe
commit fixed the bits of code where we were using directly an index
field name as an object key. This can cause errors if the field name is
a reserved JS keyword.\r\nThe fix reconfigured that part of the code by
replacing plain Objects with `Map` that doesn't suffer from this
issue.","sha":"5a2f51c347425ce34fe99a1824ae47f0c5c56f79","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Visualizations","Feature:Lens","backport:skip","v8.7.0"],"number":148118,"url":"https://github.com/elastic/kibana/pull/148118","mergeCommit":{"message":"[Lens]
gracefully handle JS Object method names as field names (#148118)\n\nThe
commit fixed the bits of code where we were using directly an index
field name as an object key. This can cause errors if the field name is
a reserved JS keyword.\r\nThe fix reconfigured that part of the code by
replacing plain Objects with `Map` that doesn't suffer from this
issue.","sha":"5a2f51c347425ce34fe99a1824ae47f0c5c56f79"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/148118","number":148118,"mergeCommit":{"message":"[Lens]
gracefully handle JS Object method names as field names (#148118)\n\nThe
commit fixed the bits of code where we were using directly an index
field name as an object key. This can cause errors if the field name is
a reserved JS keyword.\r\nThe fix reconfigured that part of the code by
replacing plain Objects with `Map` that doesn't suffer from this
issue.","sha":"5a2f51c347425ce34fe99a1824ae47f0c5c56f79"}}]}]
BACKPORT-->
This commit is contained in:
Marco Vettorello 2023-01-27 15:52:01 +01:00 committed by GitHub
parent e65fb36da1
commit fc4a6b8435
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 68 additions and 63 deletions

View file

@ -288,7 +288,7 @@ export function DimensionEditor(props: DimensionEditorProps) {
!('selectionStyle' in operationDefinition) ||
operationDefinition.selectionStyle !== 'hidden'
)
.filter(({ type }) => fieldByOperation[type]?.size || operationWithoutField.has(type))
.filter(({ type }) => fieldByOperation.get(type)?.size || operationWithoutField.has(type))
.sort((op1, op2) => {
return op1.displayName.localeCompare(op2.displayName);
})
@ -499,7 +499,7 @@ export function DimensionEditor(props: DimensionEditorProps) {
setStateWrapper(newLayer);
return;
} else if (!selectedColumn || !compatibleWithCurrentField) {
const possibleFields = fieldByOperation[operationType] || new Set();
const possibleFields = fieldByOperation.get(operationType) ?? new Set<string>();
let newLayer: FormBasedLayer;
if (possibleFields.size === 1) {

View file

@ -122,6 +122,15 @@ const fields = [
searchable: true,
exists: true,
},
// Added to test issue#148062 about the use of Object method names as fields name
...Object.getOwnPropertyNames(Object.getPrototypeOf({})).map((name) => ({
name,
displayName: name,
type: 'string',
aggregatable: true,
searchable: true,
exists: true,
})),
documentField,
];
@ -330,7 +339,7 @@ describe('FormBasedDimensionEditor', () => {
.filter('[data-test-subj="indexPattern-dimension-field"]')
.prop('options');
expect(options).toHaveLength(2);
expect(options).toHaveLength(3);
expect(options![0].label).toEqual('Records');
expect(options![1].options!.map(({ label }) => label)).toEqual([
@ -339,6 +348,11 @@ describe('FormBasedDimensionEditor', () => {
'memory',
'source',
]);
// these fields are generated to test the issue #148062 about fields that are using JS Object method names
expect(options![2].options!.map(({ label }) => label)).toEqual(
Object.getOwnPropertyNames(Object.getPrototypeOf({})).sort()
);
});
it('should hide fields that have no data', () => {

View file

@ -50,16 +50,15 @@ export function FieldSelect({
fieldIsInvalid,
markAllFieldsCompatible,
['data-test-subj']: dataTestSub,
...rest
}: FieldSelectProps) {
const { hasFieldData } = useExistingFieldsReader();
const memoizedFieldOptions = useMemo(() => {
const fields = Object.keys(operationByField).sort();
const fields = [...operationByField.keys()].sort();
const currentOperationType = incompleteOperation ?? selectedOperationType;
function isCompatibleWithCurrentOperation(fieldName: string) {
return !currentOperationType || operationByField[fieldName]!.has(currentOperationType);
return !currentOperationType || operationByField.get(fieldName)!.has(currentOperationType);
}
const [specialFields, normalFields] = partition(
@ -91,7 +90,7 @@ export function FieldSelect({
operationType:
currentOperationType && isCompatibleWithCurrentOperation(field)
? currentOperationType
: operationByField[field]!.values().next().value,
: operationByField.get(field)!.values().next().value, // TODO let's remove these non-null assertion, they are very dangerous
},
exists,
compatible,

View file

@ -12,9 +12,9 @@ import { memoizedGetAvailableOperationsByMetadata, OperationFieldTuple } from '.
import { FormBasedPrivateState } from '../types';
export interface OperationSupportMatrix {
operationByField: Partial<Record<string, Set<OperationType>>>;
operationByField: Map<string, Set<OperationType>>;
operationWithoutField: Set<OperationType>;
fieldByOperation: Partial<Record<OperationType, Set<string>>>;
fieldByOperation: Map<OperationType, Set<string>>;
}
type Props = Pick<
@ -28,37 +28,30 @@ function computeOperationMatrix(
operations: OperationFieldTuple[];
}>,
filterOperations: (operation: OperationMetadata) => boolean
) {
const filteredOperationsByMetadata = operationsByMetadata.filter((operation) =>
filterOperations(operation.operationMetaData)
);
const supportedOperationsByField: Partial<Record<string, Set<OperationType>>> = {};
const supportedOperationsWithoutField: Set<OperationType> = new Set();
const supportedFieldsByOperation: Partial<Record<OperationType, Set<string>>> = {};
filteredOperationsByMetadata.forEach(({ operations }) => {
operations.forEach((operation) => {
if (operation.type === 'field') {
if (!supportedOperationsByField[operation.field]) {
supportedOperationsByField[operation.field] = new Set();
): OperationSupportMatrix {
return operationsByMetadata
.reduce<OperationFieldTuple[]>((opsFieldTuples, { operationMetaData, operations }) => {
return filterOperations(operationMetaData)
? [...opsFieldTuples, ...operations]
: opsFieldTuples;
}, [])
.reduce<OperationSupportMatrix>(
(matrix, operation) => {
if (operation.type === 'field') {
const fieldOps = matrix.operationByField.get(operation.field) ?? new Set<OperationType>();
fieldOps.add(operation.operationType);
matrix.operationByField.set(operation.field, fieldOps);
const opFields =
matrix.fieldByOperation.get(operation.operationType) ?? new Set<string>();
opFields.add(operation.field);
matrix.fieldByOperation.set(operation.operationType, opFields);
} else {
matrix.operationWithoutField.add(operation.operationType);
}
supportedOperationsByField[operation.field]?.add(operation.operationType);
if (!supportedFieldsByOperation[operation.operationType]) {
supportedFieldsByOperation[operation.operationType] = new Set();
}
supportedFieldsByOperation[operation.operationType]?.add(operation.field);
} else {
supportedOperationsWithoutField.add(operation.operationType);
}
});
});
return {
operationByField: supportedOperationsByField,
operationWithoutField: supportedOperationsWithoutField,
fieldByOperation: supportedFieldsByOperation,
};
return matrix;
},
{ operationByField: new Map(), operationWithoutField: new Set(), fieldByOperation: new Map() }
);
}
// memoize based on latest execution. It supports multiple args

View file

@ -50,7 +50,7 @@ const getFunctionOptions = (
(column &&
hasField(column) &&
def.input === 'field' &&
operationSupportMatrix.fieldByOperation[operationType]?.has(column.sourceField)) ||
operationSupportMatrix.fieldByOperation.get(operationType)?.has(column.sourceField)) ||
(column && !hasField(column) && def.input !== 'field');
return {
@ -130,8 +130,8 @@ export const ReferenceEditor = (props: ReferenceEditorProps) => {
} = useMemo(() => {
const operationTypes: Set<OperationType> = new Set();
const operationWithoutField: Set<OperationType> = new Set();
const operationByField: Partial<Record<string, Set<OperationType>>> = {};
const fieldByOperation: Partial<Record<OperationType, Set<string>>> = {};
const operationByField: Map<string, Set<OperationType>> = new Map();
const fieldByOperation: Map<OperationType, Set<string>> = new Map();
Object.values(operationDefinitionMap)
.filter(({ hidden, allowAsReference }) => !hidden && allowAsReference)
.sort((op1, op2) => {
@ -149,12 +149,11 @@ export const ReferenceEditor = (props: ReferenceEditorProps) => {
);
if (allFields.length) {
operationTypes.add(op.type);
fieldByOperation[op.type] = new Set(allFields.map(({ name }) => name));
fieldByOperation.set(op.type, new Set(allFields.map(({ name }) => name)));
allFields.forEach((field) => {
if (!operationByField[field.name]) {
operationByField[field.name] = new Set();
}
operationByField[field.name]?.add(op.type);
const fieldOps = operationByField.get(field.name) ?? new Set<OperationType>();
fieldOps.add(op.type);
operationByField.set(field.name, fieldOps);
});
}
} else if (
@ -265,7 +264,8 @@ export const ReferenceEditor = (props: ReferenceEditorProps) => {
if (column?.operationType === operationType) {
return;
}
const possibleFieldNames = operationSupportMatrix.fieldByOperation[operationType];
const possibleFieldNames =
operationSupportMatrix.fieldByOperation.get(operationType);
const field =
column && 'sourceField' in column && possibleFieldNames?.has(column.sourceField)

View file

@ -102,7 +102,7 @@ export function FieldInputs({
// * a field of unsupported type should be removed
// * a field that has been used
// * a scripted field was used in a singular term, should be marked as invalid for multi-terms
const filteredOperationByField = Object.keys(operationSupportMatrix.operationByField)
const filteredOperationByField = [...operationSupportMatrix.operationByField.keys()]
.filter((key) => {
if (key === value) {
return true;
@ -120,9 +120,12 @@ export function FieldInputs({
}
})
.reduce<OperationSupportMatrix['operationByField']>((memo, key) => {
memo[key] = operationSupportMatrix.operationByField[key];
const fieldOps = operationSupportMatrix.operationByField.get(key);
if (fieldOps) {
memo.set(key, fieldOps);
}
return memo;
}, {});
}, new Map());
const shouldShowError = Boolean(
value &&

View file

@ -451,7 +451,7 @@ export const termsOperation: OperationDefinition<
// in single field mode, allow the automatic switch of the function to
// the most appropriate one
if (fields.length === 1) {
const possibleOperations = operationSupportMatrix.operationByField[sourcefield];
const possibleOperations = operationSupportMatrix.operationByField.get(sourcefield);
const termsSupported = possibleOperations?.has('terms');
if (!termsSupported) {
const newFieldOp = possibleOperations?.values().next().value;

View file

@ -22,14 +22,12 @@ const getFirstFocusable = (el: HTMLElement | null) => {
return firstFocusable as unknown as { focus: () => void };
};
type RefsById = Record<string, HTMLElement | null>;
export function useFocusUpdate(ids: string[]) {
const [nextFocusedId, setNextFocusedId] = useState<string | null>(null);
const [refsById, setRefsById] = useState<RefsById>({});
const [refsById, setRefsById] = useState<Map<string, HTMLElement | null>>(new Map());
useEffect(() => {
const element = nextFocusedId && refsById[nextFocusedId];
const element = nextFocusedId && refsById.get(nextFocusedId);
if (element) {
const focusable = getFirstFocusable(element);
focusable?.focus();
@ -39,10 +37,9 @@ export function useFocusUpdate(ids: string[]) {
const registerNewRef = useCallback((id, el) => {
if (el) {
setRefsById((r) => ({
...r,
[id]: el,
}));
setRefsById((refs) => {
return new Map(refs.set(id, el));
});
}
}, []);
@ -55,9 +52,8 @@ export function useFocusUpdate(ids: string[]) {
const removedIndex = ids.findIndex((l) => l === id);
setRefsById((refs) => {
const newRefsById = { ...refs };
delete newRefsById[id];
return newRefsById;
refs.delete(id);
return new Map(refs);
});
const next = removedIndex === 0 ? ids[1] : ids[removedIndex - 1];
return setNextFocusedId(next);