Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] [Security Solution][Timeline] Rebuild nested fields structure from fields response (#96187) #96915

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Apr 13, 2021

Backports the following commits to 7.x:

…elds response (elastic#96187)

* First pass at rebuilding nested object structure from fields response

* Always requests TIMELINE_CTI_FIELDS as part of request

This only works for one level of nesting; will be extending tests to
allow for multiple levels momentarily.

* Build objects from arbitrary levels of nesting

This is a recursive implementation, but recursion depth is limited to
the number of levels of nesting, with arguments reducing in size as we
go (i.e. logarithmic)

* Simplify parsing logic, perf improvements

* Order short-circuiting conditions by cost, ascending
* Simplify object building for non-nested objects from fields
  * The non-nested case is the same as the base recursive case, so
    always call our recursive function if building from .fields
* Simplify getNestedParentPath
  * We can do a few simple string comparison rather than building up
    multiple strings/arrays
* Don't call getNestedParentPath unnecessarily, only if we have a field

* Simplify if branching

By definition, nestedParentFieldName can never be equal to fieldName, which means
there are only two branches here.

* Declare/export a more accurate fields type

Each top-level field value can be either an array of leaf values
(unknown[]), or an array of nested fields.

* Remove unnecessary condition

If fieldName is null or undefined, there is no reason to search for it
in dataFields. Looking through the git history this looks to be dead
code as a result of refactoring, as opposed to a legitimate bugfix, so
I'm removing it.

* Fix failing tests

* one was a test failure due to my modifying mock data
* one may have been a legitimate bug where we don't handle a hit without
  a fields response; I need to follow up with Xavier to verify.

Co-authored-by: Kibana Machine <[email protected]>
@rylnd rylnd added the backport label Apr 13, 2021
@rylnd rylnd enabled auto-merge (squash) April 13, 2021 01:05
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@rylnd rylnd merged commit fa7b6de into elastic:7.x Apr 13, 2021
@rylnd rylnd deleted the backport/7.x/pr-96187 branch April 13, 2021 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants