Skip to content

Commit

Permalink
Merge branch 'main' into endpoint-isolate-e2e-coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
szwarckonrad committed Apr 18, 2023
2 parents 9415935 + e000247 commit ac9e110
Show file tree
Hide file tree
Showing 108 changed files with 3,899 additions and 1,015 deletions.
117 changes: 110 additions & 7 deletions dev_docs/tutorials/versioning_http_apis.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ router.get(
);
```

#### Why is this problemetic for versioning?
#### Why is this problematic for versioning?

Whenever we perform a data migration the body of this endpoint will change for all clients. This prevents us from being able to maintain past interfaces and gracefully introduce new ones.

Expand Down Expand Up @@ -119,7 +119,7 @@ router.post(
}
);
```
#### Why is this problemetic for versioning?
#### Why is this problematic for versioning?

This HTTP API currently accepts all numbers and strings as input which allows for unexpected inputs like negative numbers or non-URL friendly characters. This may break future migrations or integrations that assume your data will always be within certain parameters.

Expand All @@ -141,7 +141,7 @@ This HTTP API currently accepts all numbers and strings as input which allows fo

Adding this validation we negate the risk of unexpected values. It is not necessary to use `@kbn/config-schema`, as long as your validation mechanism provides finer grained controls than "number" or "string".

In summary: think about the acceptable paramaters for every input your HTTP API expects.
In summary: think about the acceptable parameters for every input your HTTP API expects.

### 3. Keep interfaces as "narrow" as possible

Expand Down Expand Up @@ -170,7 +170,7 @@ router.get(

The above code follows guidelines from steps 1 and 2, but it allows clients to specify ANY string by which to sort. This is a far "wider" API than we need for this endpoint.

#### Why is this problemetic for versioning?
#### Why is this problematic for versioning?

Without telemetry it is impossible to know what values clients might be passing through — and what type of sort behaviour they are expecting.

Expand Down Expand Up @@ -207,9 +207,112 @@ router.get(

The changes are:

1. New input validation accepts a known set of values. This makes our HTTP API far _narrower_ and specific to our use case. It does not matter that our `sortSchema` has the same values as our persistence schema, what matters is that we created a **translation layer** between our HTTP API and our internal schema. This faclitates easily versioning this endpoint.
1. New input validation accepts a known set of values. This makes our HTTP API far _narrower_ and specific to our use case. It does not matter that our `sortSchema` has the same values as our persistence schema, what matters is that we created a **translation layer** between our HTTP API and our internal schema. This facilitates easily versioning this endpoint.
2. **Bonus point**: we use the `escapeKuery` utility to defend against KQL injection attacks.

### 4. Use the versioned API spec
### 4. Adhere to the HTTP versioning specification

#### Choosing the right version

##### Public endpoints
Public endpoints include any endpoint that is intended for users to directly integrate with via HTTP.

Choose a date string in the format `YYYY-MM-DD`. This date should be the date that a (group) of APIs was made available.

##### Internal endpoints
Internal endpoints are all non-public endpoints (see definition above).

If you need to maintain backwards-compatibility for an internal endpoint use a single, larger-than-zero number. Ex. `1`.


#### Use the versioned router

Core exposes a versioned router that ensures your endpoint's behaviour and formatting all conforms to the versioning specification.

```typescript
router.versioned.
.post({
access: 'public', // This endpoint is intended for a public audience
path: '/api/my-app/foo/{id?}',
options: { timeout: { payload: 60000 } },
})
.addVersion(
{
version: '2023-01-01', // The public version of this API
validate: {
request: {
query: schema.object({
name: schema.maybe(schema.string({ minLength: 2, maxLength: 50 })),
}),
params: schema.object({
id: schema.maybe(schema.string({ minLength: 10, maxLength: 13 })),
}),
body: schema.object({ foo: schema.string() }),
},
response: {
200: { // In development environments, this validation will run against 200 responses
body: schema.object({ foo: schema.string() }),
},
},
},
},
async (ctx, req, res) => {
await ctx.fooService.create(req.body.foo, req.params.id, req.query.name);
return res.ok({ body: { foo: req.body.foo } });
}
)
// BREAKING CHANGE: { foo: string } => { fooString: string } in response body
.addVersion(
{
version: '2023-02-01',
validate: {
request: {
query: schema.object({
name: schema.maybe(schema.string({ minLength: 2, maxLength: 50 })),
}),
params: schema.object({
id: schema.maybe(schema.string({ minLength: 10, maxLength: 13 })),
}),
body: schema.object({ fooString: schema.string() }),
},
response: {
200: {
body: schema.object({ fooName: schema.string() }),
},
},
},
},
async (ctx, req, res) => {
await ctx.fooService.create(req.body.fooString, req.params.id, req.query.name);
return res.ok({ body: { fooName: req.body.fooString } });
}
)
// BREAKING CHANGES: Enforce min/max length on fooString
.addVersion(
{
version: '2023-03-01',
validate: {
request: {
query: schema.object({
name: schema.maybe(schema.string({ minLength: 2, maxLength: 50 })),
}),
params: schema.object({
id: schema.maybe(schema.string({ minLength: 10, maxLength: 13 })),
}),
body: schema.object({ fooString: schema.string({ minLength: 0, maxLength: 1000 }) }),
},
response: {
200: {
body: schema.object({ fooName: schema.string() }),
},
},
},
},
async (ctx, req, res) => {
await ctx.fooService.create(req.body.fooString, req.params.id, req.query.name);
return res.ok({ body: { fooName: req.body.fooString } });
}
```
_Under construction, check back here soon!_
#### Additional reading
For a more details on the versioning specification see [this document](https://docs.google.com/document/d/1YpF6hXIHZaHvwNaQAxWFzexUF1nbqACTtH2IfDu0ldA/edit?usp=sharing).
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pageLoadAssetSize:
banners: 17946
bfetch: 22837
canvas: 1066647
cases: 144442
cases: 170000
charts: 55000
cloud: 21076
cloudChat: 19894
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { context } from './context';
/**
* An object representing an uploaded file
*/
interface UploadedFile<Meta = unknown> {
export interface UploadedFile<Meta = unknown> {
/**
* The ID that was generated for the uploaded file
*/
Expand Down
118 changes: 87 additions & 31 deletions x-pack/packages/ml/url_state/src/url_state.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,18 @@
* 2.0.
*/

import React, { FC } from 'react';
import React, { useEffect, type FC } from 'react';
import { render, act } from '@testing-library/react';
import { parseUrlState, useUrlState, UrlStateProvider } from './url_state';
import { MemoryRouter } from 'react-router-dom';

const mockHistoryPush = jest.fn();
import { parseUrlState, useUrlState, UrlStateProvider } from './url_state';

jest.mock('react-router-dom', () => ({
useHistory: () => ({
push: mockHistoryPush,
}),
useLocation: () => ({
search:
"?_a=(mlExplorerFilter:(),mlExplorerSwimlane:(viewByFieldName:action),query:(query_string:(analyze_wildcard:!t,query:'*')))&_g=(ml:(jobIds:!(dec-2)),refreshInterval:(display:Off,pause:!f,value:0),time:(from:'2019-01-01T00:03:40.000Z',mode:absolute,to:'2019-08-30T11:55:07.000Z'))&savedSearchId=571aaf70-4c88-11e8-b3d7-01146121b73d",
}),
}));
const mockHistoryInitialState =
"?_a=(mlExplorerFilter:(),mlExplorerSwimlane:(viewByFieldName:action),query:(query_string:(analyze_wildcard:!t,query:'*')))&_g=(ml:(jobIds:!(dec-2)),refreshInterval:(display:Off,pause:!f,value:0),time:(from:'2019-01-01T00:03:40.000Z',mode:absolute,to:'2019-08-30T11:55:07.000Z'))&savedSearchId=571aaf70-4c88-11e8-b3d7-01146121b73d";

describe('getUrlState', () => {
test('properly decode url with _g and _a', () => {
expect(
parseUrlState(
"?_a=(mlExplorerFilter:(),mlExplorerSwimlane:(viewByFieldName:action),query:(query_string:(analyze_wildcard:!t,query:'*')))&_g=(ml:(jobIds:!(dec-2)),refreshInterval:(display:Off,pause:!t,value:0),time:(from:'2019-01-01T00:03:40.000Z',mode:absolute,to:'2019-08-30T11:55:07.000Z'))&savedSearchId=571aaf70-4c88-11e8-b3d7-01146121b73d"
)
).toEqual({
expect(parseUrlState(mockHistoryInitialState)).toEqual({
_a: {
mlExplorerFilter: {},
mlExplorerSwimlane: {
Expand All @@ -46,7 +35,7 @@ describe('getUrlState', () => {
},
refreshInterval: {
display: 'Off',
pause: true,
pause: false,
value: 0,
},
time: {
Expand All @@ -61,29 +50,96 @@ describe('getUrlState', () => {
});

describe('useUrlState', () => {
beforeEach(() => {
mockHistoryPush.mockClear();
it('pushes a properly encoded search string to history', () => {
const TestComponent: FC = () => {
const [appState, setAppState] = useUrlState('_a');

useEffect(() => {
setAppState(parseUrlState(mockHistoryInitialState)._a);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return (
<>
<button onClick={() => setAppState({ query: 'my-query' })}>ButtonText</button>
<div data-test-subj="appState">{JSON.stringify(appState?.query)}</div>
</>
);
};

const { getByText, getByTestId } = render(
<MemoryRouter>
<UrlStateProvider>
<TestComponent />
</UrlStateProvider>
</MemoryRouter>
);

expect(getByTestId('appState').innerHTML).toBe(
'{"query_string":{"analyze_wildcard":true,"query":"*"}}'
);

act(() => {
getByText('ButtonText').click();
});

expect(getByTestId('appState').innerHTML).toBe('"my-query"');
});

test('pushes a properly encoded search string to history', () => {
it('updates both _g and _a state successfully', () => {
const TestComponent: FC = () => {
const [, setUrlState] = useUrlState('_a');
return <button onClick={() => setUrlState({ query: {} })}>ButtonText</button>;
const [globalState, setGlobalState] = useUrlState('_g');
const [appState, setAppState] = useUrlState('_a');

useEffect(() => {
setGlobalState({ time: 'initial time' });
setAppState({ query: 'initial query' });
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return (
<>
<button onClick={() => setGlobalState({ time: 'now-15m' })}>GlobalStateButton1</button>
<button onClick={() => setGlobalState({ time: 'now-5y' })}>GlobalStateButton2</button>
<button onClick={() => setAppState({ query: 'the updated query' })}>
AppStateButton
</button>
<div data-test-subj="globalState">{globalState?.time}</div>
<div data-test-subj="appState">{appState?.query}</div>
</>
);
};

const { getByText } = render(
<UrlStateProvider>
<TestComponent />
</UrlStateProvider>
const { getByText, getByTestId } = render(
<MemoryRouter>
<UrlStateProvider>
<TestComponent />
</UrlStateProvider>
</MemoryRouter>
);

expect(getByTestId('globalState').innerHTML).toBe('initial time');
expect(getByTestId('appState').innerHTML).toBe('initial query');

act(() => {
getByText('ButtonText').click();
getByText('GlobalStateButton1').click();
});

expect(mockHistoryPush).toHaveBeenCalledWith({
search:
'_a=%28mlExplorerFilter%3A%28%29%2CmlExplorerSwimlane%3A%28viewByFieldName%3Aaction%29%2Cquery%3A%28%29%29&_g=%28ml%3A%28jobIds%3A%21%28dec-2%29%29%2CrefreshInterval%3A%28display%3AOff%2Cpause%3A%21f%2Cvalue%3A0%29%2Ctime%3A%28from%3A%272019-01-01T00%3A03%3A40.000Z%27%2Cmode%3Aabsolute%2Cto%3A%272019-08-30T11%3A55%3A07.000Z%27%29%29&savedSearchId=571aaf70-4c88-11e8-b3d7-01146121b73d',
expect(getByTestId('globalState').innerHTML).toBe('now-15m');
expect(getByTestId('appState').innerHTML).toBe('initial query');

act(() => {
getByText('AppStateButton').click();
});

expect(getByTestId('globalState').innerHTML).toBe('now-15m');
expect(getByTestId('appState').innerHTML).toBe('the updated query');

act(() => {
getByText('GlobalStateButton2').click();
});

expect(getByTestId('globalState').innerHTML).toBe('now-5y');
expect(getByTestId('appState').innerHTML).toBe('the updated query');
});
});
15 changes: 13 additions & 2 deletions x-pack/packages/ml/url_state/src/url_state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,21 @@ export const UrlStateProvider: FC = ({ children }) => {
const history = useHistory();
const { search: searchString } = useLocation();

const searchStringRef = useRef<string>(searchString);

useEffect(() => {
searchStringRef.current = searchString;
}, [searchString]);

const setUrlState: SetUrlState = useCallback(
(
accessor: Accessor,
attribute: string | Dictionary<any>,
value?: any,
replaceState?: boolean
) => {
const prevSearchString = searchString;
const prevSearchString = searchStringRef.current;

const urlState = parseUrlState(prevSearchString);
const parsedQueryString = parse(prevSearchString, { sort: false });

Expand Down Expand Up @@ -142,6 +149,10 @@ export const UrlStateProvider: FC = ({ children }) => {

if (oldLocationSearchString !== newLocationSearchString) {
const newSearchString = stringify(parsedQueryString, { sort: false });
// Another `setUrlState` call could happen before the updated
// `searchString` gets propagated via `useLocation` therefore
// we update the ref right away too.
searchStringRef.current = newSearchString;
if (replaceState) {
history.replace({ search: newSearchString });
} else {
Expand All @@ -154,7 +165,7 @@ export const UrlStateProvider: FC = ({ children }) => {
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[searchString]
[]
);

return <Provider value={{ searchString, setUrlState }}>{children}</Provider>;
Expand Down
Loading

0 comments on commit ac9e110

Please sign in to comment.