Skip to content

Commit

Permalink
use fetch instead of core.http and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
pgayvallet committed May 31, 2021
1 parent 5c8c499 commit 3679e94
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 24 deletions.
10 changes: 3 additions & 7 deletions src/plugins/newsfeed/public/lib/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import { driverInstanceMock, storageInstanceMock } from './api.test.mocks';
import moment from 'moment';
import { httpServiceMock } from '../../../../core/public/mocks';
import { getApi } from './api';
import { TestScheduler } from 'rxjs/testing';
import { FetchResult, NewsfeedPluginBrowserConfig } from '../types';
Expand Down Expand Up @@ -40,10 +39,7 @@ const createFetchResult = (parts: Partial<FetchResult>): FetchResult => ({
});

describe('getApi', () => {
let http: ReturnType<typeof httpServiceMock.createSetupContract>;

beforeEach(() => {
http = httpServiceMock.createSetupContract();
driverInstanceMock.shouldFetch.mockReturnValue(true);
});

Expand All @@ -64,7 +60,7 @@ describe('getApi', () => {
a: createFetchResult({ feedItems: ['item' as any] }),
})
);
const api = getApi(http, createConfig(1000), kibanaVersion, newsfeedId);
const api = getApi(createConfig(1000), kibanaVersion, newsfeedId);

expectObservable(api.fetchResults$.pipe(take(1))).toBe('(a|)', {
a: createFetchResult({
Expand All @@ -87,7 +83,7 @@ describe('getApi', () => {
a: createFetchResult({ feedItems: ['item' as any] }),
})
);
const api = getApi(http, createConfig(2), kibanaVersion, newsfeedId);
const api = getApi(createConfig(2), kibanaVersion, newsfeedId);

expectObservable(api.fetchResults$.pipe(take(2))).toBe('a-(b|)', {
a: createFetchResult({
Expand Down Expand Up @@ -115,7 +111,7 @@ describe('getApi', () => {
a: createFetchResult({}),
})
);
const api = getApi(http, createConfig(10), kibanaVersion, newsfeedId);
const api = getApi(createConfig(10), kibanaVersion, newsfeedId);

expectObservable(api.fetchResults$.pipe(take(2))).toBe('a--(b|)', {
a: createFetchResult({
Expand Down
4 changes: 1 addition & 3 deletions src/plugins/newsfeed/public/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import { combineLatest, Observable, timer, of } from 'rxjs';
import { map, catchError, filter, mergeMap, tap } from 'rxjs/operators';
import { i18n } from '@kbn/i18n';
import { HttpSetup } from 'src/core/public';
import { FetchResult, NewsfeedPluginBrowserConfig } from '../types';
import { NewsfeedApiDriver } from './driver';
import { NewsfeedStorage } from './storage';
Expand Down Expand Up @@ -39,7 +38,6 @@ export interface NewsfeedApi {
* Computes hasNew value from new item hashes saved in localStorage
*/
export function getApi(
http: HttpSetup,
config: NewsfeedPluginBrowserConfig,
kibanaVersion: string,
newsfeedId: string
Expand All @@ -53,7 +51,7 @@ export function getApi(
const results$ = timer(0, mainInterval).pipe(
filter(() => driver.shouldFetch()),
mergeMap(() =>
driver.fetchNewsfeedItems(http, config.service).pipe(
driver.fetchNewsfeedItems(config.service).pipe(
catchError((err) => {
window.console.error(err);
return of({
Expand Down
12 changes: 12 additions & 0 deletions src/plugins/newsfeed/public/lib/driver.test.mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export const convertItemsMock = jest.fn();
jest.doMock('./convert_items', () => ({
convertItems: convertItemsMock,
}));
91 changes: 91 additions & 0 deletions src/plugins/newsfeed/public/lib/driver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
* Side Public License, v 1.
*/

import { convertItemsMock } from './driver.test.mocks';
// @ts-expect-error
import fetchMock from 'fetch-mock/es5/client';
import { take } from 'rxjs/operators';
import { NewsfeedApiDriver } from './driver';
import { storageMock } from './storage.mock';

Expand All @@ -20,6 +24,16 @@ describe('NewsfeedApiDriver', () => {
beforeEach(() => {
storage = storageMock.create();
driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage);
convertItemsMock.mockReturnValue([]);
});

afterEach(() => {
fetchMock.reset();
convertItemsMock.mockReset();
});

afterAll(() => {
fetchMock.restore();
});

describe('shouldFetch', () => {
Expand All @@ -39,4 +53,81 @@ describe('NewsfeedApiDriver', () => {
expect(driver.shouldFetch()).toBe(false);
});
});

describe('fetchNewsfeedItems', () => {
it('calls `window.fetch` with the correct parameters', async () => {
fetchMock.get('*', { items: [] });
await driver
.fetchNewsfeedItems({
urlRoot: 'http://newsfeed.com',
pathTemplate: '/{VERSION}/news',
})
.pipe(take(1))
.toPromise();

expect(fetchMock.lastUrl()).toEqual('http://newsfeed.com/8.0.0/news');
expect(fetchMock.lastOptions()).toEqual({
method: 'GET',
});
});

it('calls `convertItems` with the correct parameters', async () => {
fetchMock.get('*', { items: ['foo', 'bar'] });

await driver
.fetchNewsfeedItems({
urlRoot: 'http://newsfeed.com',
pathTemplate: '/{VERSION}/news',
})
.pipe(take(1))
.toPromise();

expect(convertItemsMock).toHaveBeenCalledTimes(1);
expect(convertItemsMock).toHaveBeenCalledWith(['foo', 'bar'], userLanguage);
});

it('calls `storage.setFetchedItems` with the correct parameters', async () => {
fetchMock.get('*', { items: [] });
convertItemsMock.mockReturnValue([
{ id: '1', hash: 'hash1' },
{ id: '2', hash: 'hash2' },
]);

await driver
.fetchNewsfeedItems({
urlRoot: 'http://newsfeed.com',
pathTemplate: '/{VERSION}/news',
})
.pipe(take(1))
.toPromise();

expect(storage.setFetchedItems).toHaveBeenCalledTimes(1);
expect(storage.setFetchedItems).toHaveBeenCalledWith(['hash1', 'hash2']);
});

it('returns the expected values', async () => {
fetchMock.get('*', { items: [] });
const feedItems = [
{ id: '1', hash: 'hash1' },
{ id: '2', hash: 'hash2' },
];
convertItemsMock.mockReturnValue(feedItems);
storage.setFetchedItems.mockReturnValue(true);

const result = await driver
.fetchNewsfeedItems({
urlRoot: 'http://newsfeed.com',
pathTemplate: '/{VERSION}/news',
})
.pipe(take(1))
.toPromise();

expect(result).toEqual({
error: null,
kibanaVersion,
hasNew: true,
feedItems,
});
});
});
});
21 changes: 12 additions & 9 deletions src/plugins/newsfeed/public/lib/driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import moment from 'moment';
import { HttpSetup } from 'kibana/public';
import * as Rx from 'rxjs';
import { NEWSFEED_DEFAULT_SERVICE_BASE_URL } from '../../common/constants';
import { ApiItem, FetchResult, NewsfeedPluginBrowserConfig } from '../types';
Expand All @@ -16,6 +15,10 @@ import type { NewsfeedStorage } from './storage';

type ApiConfig = NewsfeedPluginBrowserConfig['service'];

interface NewsfeedResponse {
items: ApiItem[];
}

export class NewsfeedApiDriver {
private readonly kibanaVersion: string;
private readonly loadedTime = moment().utc(); // the date is compared to time in UTC format coming from the service
Expand Down Expand Up @@ -47,18 +50,18 @@ export class NewsfeedApiDriver {
return duration.asMilliseconds() > this.fetchInterval;
}

fetchNewsfeedItems(http: HttpSetup, config: ApiConfig): Rx.Observable<FetchResult> {
fetchNewsfeedItems(config: ApiConfig): Rx.Observable<FetchResult> {
const urlPath = config.pathTemplate.replace('{VERSION}', this.kibanaVersion);
const fullUrl = (config.urlRoot || NEWSFEED_DEFAULT_SERVICE_BASE_URL) + urlPath;
const request = new Request(fullUrl, {
method: 'GET',
});

return Rx.from(
http
.fetch(fullUrl, {
method: 'GET',
})
.then(({ items }: { items: ApiItem[] }) => {
return this.convertResponse(items);
})
window.fetch(request).then(async (response) => {
const { items } = (await response.json()) as NewsfeedResponse;
return this.convertResponse(items);
})
);
}

Expand Down
8 changes: 3 additions & 5 deletions src/plugins/newsfeed/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class NewsfeedPublicPlugin
}

public start(core: CoreStart) {
const api = this.createNewsfeedApi(core, this.config, NewsfeedApiEndpoint.KIBANA);
const api = this.createNewsfeedApi(this.config, NewsfeedApiEndpoint.KIBANA);
core.chrome.navControls.registerRight({
order: 1000,
mount: (target) => this.mount(api, target),
Expand All @@ -56,7 +56,7 @@ export class NewsfeedPublicPlugin
pathTemplate: `/${endpoint}/v{VERSION}.json`,
},
});
const { fetchResults$ } = this.createNewsfeedApi(core, config, endpoint);
const { fetchResults$ } = this.createNewsfeedApi(config, endpoint);
return fetchResults$;
},
};
Expand All @@ -67,12 +67,10 @@ export class NewsfeedPublicPlugin
}

private createNewsfeedApi(
core: CoreStart,
config: NewsfeedPluginBrowserConfig,
newsfeedId: NewsfeedApiEndpoint
): NewsfeedApi {
const { http } = core;
const api = getApi(http, config, this.kibanaVersion, newsfeedId);
const api = getApi(config, this.kibanaVersion, newsfeedId);
return {
markAsRead: api.markAsRead,
fetchResults$: api.fetchResults$.pipe(
Expand Down

0 comments on commit 3679e94

Please sign in to comment.