Skip to content

Commit

Permalink
Add Accept header in FetchProvider. (#10481)
Browse files Browse the repository at this point in the history
* Add `Accept` header in `FetchProvider`.

This change is fixing an issue introduced in the transition to the
native fetch API, which left out setting the `Accept` header in the
generated request. The value for the header was tracked in a private
field of the `Builder` class, but never set for the client.

* Adding cast.
  • Loading branch information
dennisoelkers committed Apr 22, 2021
1 parent 867f2a9 commit 3526b35
Show file tree
Hide file tree
Showing 5 changed files with 379 additions and 114 deletions.
1 change: 1 addition & 0 deletions graylog2-web-interface/package.json
Expand Up @@ -120,6 +120,7 @@
"@babel/preset-react": "7.12.13",
"@testing-library/user-event": "^12.6.3",
"@types/chroma-js": "^2.1.3",
"@types/express": "^4.17.11",
"@types/lodash": "^4.14.165",
"@types/plotly.js": "1.54.8",
"@types/react-plotly.js": "^2.2.4",
Expand Down
34 changes: 26 additions & 8 deletions graylog2-web-interface/src/logic/rest/FetchProvider.test.ts
Expand Up @@ -17,7 +17,7 @@
import express from 'express';
import nodeFetch from 'node-fetch';

import fetch from './FetchProvider';
import fetch, { fetchFile } from './FetchProvider';

jest.unmock('./FetchProvider');

Expand Down Expand Up @@ -51,32 +51,50 @@ const setUpServer = () => {
res.status(204).end();
});

app.post('/failIfWrongAcceptHeader', (req, res) => {
if (req.accepts().includes('text/csv')) {
res.send('foo,bar,baz');
} else {
res.status(500).end();
}
});

return app.listen(PORT, () => {});
};

describe('FetchProvider', () => {
let server;
let server: ReturnType<typeof setUpServer>;
let baseUrl;

beforeAll(() => {
server = setUpServer();
// eslint-disable-next-line global-require
window.fetch = nodeFetch;

// @ts-ignore Types do not match actual result for some reason
const { port } = server.address();
baseUrl = `http://localhost:${port}`;
});

afterAll(() => {
server.close();
});

it.each([
['a GET with json', 'GET', 'test1', { text: 'test' }],
['a POST with json', 'POST', 'test2', { text: 'test' }],
['a POST with text', 'POST', 'test3', 'uuid-beef-feed'],
['a POST without content', 'POST', 'test4', null],
['a DELETE without content and status 204', 'DELETE', 'test5', null],
['GET with json', 'GET', 'test1', { text: 'test' }],
['POST with json', 'POST', 'test2', { text: 'test' }],
['POST with text', 'POST', 'test3', 'uuid-beef-feed'],
['POST without content', 'POST', 'test4', null],
['DELETE without content and status 204', 'DELETE', 'test5', null],
])('should receive a %s', async (text, method, url, expectedResponse) => {
return fetch(method, `http://localhost:${server.address().port}/${url}`, undefined).then((response) => {
return fetch(method, `${baseUrl}/${url}`, undefined).then((response) => {
expect(response).toStrictEqual(expectedResponse);
});
});

it('sets correct accept header', async () => {
const result = await fetchFile('POST', `${baseUrl}/failIfWrongAcceptHeader`, {}, 'text/csv');

expect(result).toEqual('foo,bar,baz');
});
});
11 changes: 10 additions & 1 deletion graylog2-web-interface/src/logic/rest/FetchProvider.ts
Expand Up @@ -57,6 +57,11 @@ const onServerError = (error, onUnauthorized = defaultOnUnauthorizedError) => {

const maybeStringify = (body: any) => (body && typeof body !== 'string' ? JSON.stringify(body) : body);

type RequestHeaders = {
Accept?: string,
'Content-Type'?: string,
};

export class Builder {
private options = {};

Expand Down Expand Up @@ -185,10 +190,14 @@ export class Builder {
}

build() {
const headers = this.body
const headers: RequestHeaders = this.body
? { ...this.options, 'Content-Type': this.body.mimeType }
: this.options;

if (this.accept) {
headers.Accept = this.accept;
}

return window.fetch(this.url, {
method: this.method,
headers,
Expand Down
Expand Up @@ -145,7 +145,7 @@ const WidgetFocusProvider = ({ children }: { children: React.ReactNode }): React
};

const unsetWidgetEditing = () => updateFocusQueryParams({
id: focusUriParams.focusing && focusUriParams.id ? focusUriParams.id : undefined,
id: focusUriParams.focusing && focusUriParams.id ? focusUriParams.id as string : undefined,
editing: false,
focusing: focusUriParams.focusing,
});
Expand Down

0 comments on commit 3526b35

Please sign in to comment.