[Security Solution] Organize data fetching hooks in one place #136962

xcrzx · 2022-07-22T15:19:13Z

Related to: #136957

Summary

Organize react-query hooks that fetch data similarly to how we organize API clients. The idea is to make them more visible and to give developers a hint that fetching data using react-query hooks is the preferred way.

Coallocalte query/mutation hooks together; grouped by domain
Queries should be highly reusable, i.e., they should not contain any business-related params, and all options should be configurable.
Queries should maintain and enforce an effective query key hierarchy (read more on that https://tkdodo.eu/blog/effective-react-query-keys).
Mutation queries should automatically invalidate or update associated cache entries.

Query example:

export const useMyQuery = (queryArgs: MyQueryArgs, queryOptions: UseQueryOptions) => {
  return useQuery(
    [TOP_LEVEL_QUERY_KEY, MORE_SPECIFIC_QUERY_KEY, queryArgs],
    async ({ signal }) => {
      return fetchData({ signal, ...queryArgs });
    },
    queryOptions
  );
};

elasticmachine · 2022-07-22T15:19:16Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

elasticmachine · 2022-07-22T15:19:16Z

Pinging @elastic/security-solution (Team: SecuritySolution)

…toring Rule Management (#142950) **Partially addresses:** #138600, #92169, #138606 **Addresses:** #136957, #136962, #138614 ## Summary In this PR we are: - Splitting the Detection Engine into subdomains ([ticket](#138600)). Every subdomain got its own folder under `detection_engine`, and we moved some (not all) code into them. More on that is below. New subdomains introduced: - `fleet_integrations` - `prebuilt_rules` - `rule_actions_legacy` - `rule_exceptions` - `rule_management` - `rule_preview` - `rule_schema` - `rule_creation_ui` - `rule_details_ui` - `rule_management_ui` - `rule_exceptions_ui` - Updating the CODEOWNERS file accordingly. - Refactoring the Rule Management page and the Rules table. Our main focus was on the way how we communicate with the API endpoints, how we cache and invalidate the fetched data, and how this code is organized in the codebase. More on that is below. - Increasing the bundle size limit. This is going to be decreased back in a follow-up PR ([ticket](#143532)) ## Restructuring folders into subdomains For the background and problem statement, please refer to #138600 We were focusing on code that is closely related to the Rules area: either owned by us de facto (we work on it) or owned by us de jure (according to the CODEOWNERS file). Or goal was to explicitly extract code that we don't own de facto into separate subdomains, transfer ownership to other area teams, and reflect this in the CODEOWNERS file. On the other hand, we wanted the code that we own to also be organized in clear subdomains that we could easily own via CODEOWNERS. We didn't touch the code that is already explicitly owned by other area teams, e.g. `x-pack/plugins/security_solution/server/lib/detection_engine/rule_types`. This is a draft "domain map" - an architectural diagram that shows how the Detection Engine _could_ be split into subdomains. It's more a TO-BE idea/aspiration rather than an AS-IS statement. Any feedback, critiques, and suggestions would be extremely appreciated! <img width="2592" alt="Screenshot 2022-10-18 at 16 08 40" src="https://user-images.githubusercontent.com/7359339/196453965-b65f5b49-9a33-4d90-bb48-1347e9576223.png"> It shows the flow of dependencies between subdomains and proposes some rules: - The whole graph of dependencies between all subdomains should be a DAG. There should not be bi-directional or circular dependencies between them. - **Generic subdomains** represent some general knowledge that can be used/applied outside of the Detection Engine. - Can depend on some generic kbn packages, npm packages or utils. - Can't depend on any other Detection Engine subdomains. - **Crosscutting subdomains** represent some code that can be common to / shared between many other subdomains. This could be some very common domain models and API schemas. - Can depend on generic subdomains. - Can depend on other crosscutting subdomains (dependencies between them must form a DAG). - Can't depend on core or UI subdomains. - **Core subdomains** contain most of the "meat" of the Detection Engine: domain models, server-side and client-side business logic, server-side API endpoints, client-side UI (potentially shareable between several pages). - Can depend on crosscutting and generic subdomains. - Can depend on other core subdomains (dependencies between them must form a DAG). - Can't depend on UI subdomains. - **UI subdomains** contain the implementation of pages related to the Detection Engine. Every page can easily depend on several core subdomains, so these subdomain are on top of everything. - Can depend on any other subdomains. Dependencies must form a DAG. Dashed lines show some existing dependencies that we think should be eliminated. Ownership TO-BE is color-coded. We updated the CODEOWNERS file according to the new folders. The folder restructuring is not 100% finished but we did a big part of it. Most of the FE code continues to live in legacy folders, e.g. see `x-pack/plugins/security_solution/public/detections`. So this work is to be continued... ## Refactoring of Rule Management FE - [x] #136957 For effective HTTP requests caching and deduplication, we've migrated all data fetching logic to `useQuery` and `useMutation` hooks from `react-query`. That allowed us to introduce the following improvements to our codebase: * All outgoing HTTP requests are now automatically deduplicated. That means that data fetching hooks like `useRule` could be used on any level in the component tree to access response data directly. So, no need to put the hook on the top level anymore and use prop-drilling to make the response data available to all children components that require it. * All HTTP responses are now cached with the default TTL of 5 minutes—no more redundant requests. With a hot cache, transitions to some pages now happen immediately. - [x] #136962 Data fetching hooks of the Rules Area are now organized in one place. `security_solution/public/detection_engine/rule_management/api/hooks` contains abstraction layer on top of the Kibana's HTTP client. All data fetching should happen exclusively through that layer to ensure that: * Mutation queries automatically invalidate associated cache entries. * Optimistic updates or updates from mutation responses could be implemented centrally where possible. - [x] #92169 From some of the Rule Management components, logic was extracted to hooks located in `security_solution/public/detection_engine/rule_management/logic`. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

banderror · 2022-10-21T14:50:12Z

@xcrzx We now have this for the rule_management subdomain, but in other subdomains it's not done. Let's decide what we're going to do with this ticket.

xcrzx · 2022-10-25T09:08:42Z

@xcrzx We now have this for the rule_management subdomain, but in other subdomains it's not done. Let's decide what we're going to do with this ticket.

I've created an additional ticket to restrict direct usage of the API client, so work on our end is mainly done. I think we could close this ticket.

As for other subdomains, we can share knowledge and show in more detail how react-query helps us organize our code so other areas can decide whether that approach suits them well and if they want to follow the pattern.

xcrzx · 2022-10-25T09:11:09Z

Implemented in #142950

@banderror I'm closing this issue, but feel free to reopen it if there's something else that could be done.

This was referenced Jul 22, 2022

[Security Solution] Migrate hooks that fetch data and mutate server state to react-query #136957

Closed

[Security Solution][Detections] Extended rule execution logging to Event Log #126063

Merged

banderror changed the title ~~Organize data fetching hooks in one place~~ [Security Solution][Detections] Organize data fetching hooks in one place Jul 26, 2022

xcrzx self-assigned this Oct 11, 2022

banderror mentioned this issue Oct 13, 2022

[Security Solution] Restructuring folders of Detection Engine + refactoring Rule Management #142950

Merged

4 tasks

banderror changed the title ~~[Security Solution][Detections] Organize data fetching hooks in one place~~ [Security Solution] Organize data fetching hooks in one place Oct 21, 2022

xcrzx mentioned this issue Oct 25, 2022

[Security Solution] Add an ESLint rule to prohibit direct API client usage #143923

Open

xcrzx closed this as completed Oct 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Organize data fetching hooks in one place #136962

[Security Solution] Organize data fetching hooks in one place #136962

xcrzx commented Jul 22, 2022

elasticmachine commented Jul 22, 2022

elasticmachine commented Jul 22, 2022

banderror commented Oct 21, 2022

xcrzx commented Oct 25, 2022 •

edited

Loading

xcrzx commented Oct 25, 2022

[Security Solution] Organize data fetching hooks in one place #136962

[Security Solution] Organize data fetching hooks in one place #136962

Comments

xcrzx commented Jul 22, 2022

Summary

elasticmachine commented Jul 22, 2022

elasticmachine commented Jul 22, 2022

banderror commented Oct 21, 2022

xcrzx commented Oct 25, 2022 • edited Loading

xcrzx commented Oct 25, 2022

xcrzx commented Oct 25, 2022 •

edited

Loading