Skip to content

Commit

Permalink
unskip cache and abort related tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dosant
Dosant committed Jul 25, 2022
1 parent cdbcc7d commit 01f4b5d
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 33 deletions.
84 changes: 56 additions & 28 deletions src/plugins/data/public/search/search_interceptor/search_interceptor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ describe('SearchInterceptor', () => {
expect(error.mock.calls[0][0]).toBeInstanceOf(AbortError);
});

test.skip('should DELETE a running async search on abort', async () => {
test('should DELETE a running async search on abort', async () => {
const responses = [
{
time: 10,
Expand Down Expand Up @@ -362,7 +362,7 @@ describe('SearchInterceptor', () => {
expect(mockCoreSetup.http.delete).not.toHaveBeenCalled();
});

test.skip('should DELETE a running async search on async timeout after first response', async () => {
test('should DELETE a running async search on async timeout after first response', async () => {
const responses = [
{
time: 10,
Expand Down Expand Up @@ -404,7 +404,7 @@ describe('SearchInterceptor', () => {
expect(mockCoreSetup.http.delete).toHaveBeenCalledTimes(1);
});

test.skip('should DELETE a running async search on async timeout on error from fetch', async () => {
test('should DELETE a running async search on async timeout on error from fetch', async () => {
const responses = [
{
time: 10,
Expand Down Expand Up @@ -447,7 +447,7 @@ describe('SearchInterceptor', () => {
expect(mockCoreSetup.http.delete).toHaveBeenCalledTimes(1);
});

test.skip('should NOT DELETE a running SAVED async search on abort', async () => {
test('should NOT DELETE a running SAVED async search on abort', async () => {
const sessionId = 'sessionId';
sessionService.isCurrentSession.mockImplementation((_sessionId) => _sessionId === sessionId);
const responses = [
Expand Down Expand Up @@ -907,16 +907,22 @@ describe('SearchInterceptor', () => {
expect(fetchMock).toBeCalledTimes(2);
});

// TODO: changed, not cached searches are not tracked, double check edge cases.
test.skip('should track searches that come from cache', async () => {
test('should not track searches that come from cache', async () => {
mockFetchImplementation(partialCompleteResponse);
sessionService.isCurrentSession.mockImplementation(
(_sessionId) => _sessionId === sessionId
);
sessionService.getSessionId.mockImplementation(() => sessionId);

// const untrack = jest.fn();
// sessionService.trackSearch.mockImplementation(() => untrack);
const completeSearch = jest.fn();

sessionService.trackSearch.mockImplementation((params) => ({
complete: completeSearch,
error: jest.fn(),
beforePoll: jest.fn(() => {
return [{ isSearchStored: false }, () => {}];
}),
}));

const req = {
params: {
Expand All @@ -929,14 +935,15 @@ describe('SearchInterceptor', () => {
response.subscribe({ next, error, complete });
response2.subscribe({ next, error, complete });
await timeTravel(10);

expect(fetchMock).toBeCalledTimes(1);
expect(sessionService.trackSearch).toBeCalledTimes(2);
// expect(untrack).not.toBeCalled();
expect(sessionService.trackSearch).toBeCalledTimes(1);
expect(completeSearch).not.toBeCalled();
await timeTravel(300);
// Should be called only 2 times (once per partial response)
expect(fetchMock).toBeCalledTimes(2);
expect(sessionService.trackSearch).toBeCalledTimes(2);
// expect(untrack).toBeCalledTimes(2);
expect(sessionService.trackSearch).toBeCalledTimes(1);
expect(completeSearch).toBeCalledTimes(1);

expect(next).toBeCalledTimes(4);
expect(error).toBeCalledTimes(0);
Expand Down Expand Up @@ -1115,15 +1122,22 @@ describe('SearchInterceptor', () => {
expect(complete2).toBeCalledTimes(1);
});

test.skip('aborting a running first search shouldnt clear cache', async () => {
test('aborting a running first search shouldnt clear cache', async () => {
mockFetchImplementation(partialCompleteResponse);
sessionService.isCurrentSession.mockImplementation(
(_sessionId) => _sessionId === sessionId
);
sessionService.getSessionId.mockImplementation(() => sessionId);

// const untrack = jest.fn();
// sessionService.trackSearch.mockImplementation(() => untrack);
const completeSearch = jest.fn();

sessionService.trackSearch.mockImplementation((params) => ({
complete: completeSearch,
error: jest.fn(),
beforePoll: jest.fn(() => {
return [{ isSearchStored: false }, () => {}];
}),
}));

const req = {
params: {
Expand Down Expand Up @@ -1157,9 +1171,9 @@ describe('SearchInterceptor', () => {
abortController.abort();

await timeTravel(300);
// Both searches should be tracked and untracked
expect(sessionService.trackSearch).toBeCalledTimes(2);
// expect(untrack).toBeCalledTimes(2);
// Only first searches should be tracked and untracked
expect(sessionService.trackSearch).toBeCalledTimes(1);
expect(completeSearch).toBeCalledTimes(1);

// First search should error
expect(next).toBeCalledTimes(1);
Expand All @@ -1175,15 +1189,22 @@ describe('SearchInterceptor', () => {
expect(fetchMock).toBeCalledTimes(2);
});

test.skip('aborting a running second search shouldnt clear cache', async () => {
test('aborting a running second search shouldnt clear cache', async () => {
mockFetchImplementation(partialCompleteResponse);
sessionService.isCurrentSession.mockImplementation(
(_sessionId) => _sessionId === sessionId
);
sessionService.getSessionId.mockImplementation(() => sessionId);

// const untrack = jest.fn();
// sessionService.trackSearch.mockImplementation(() => untrack);
const completeSearch = jest.fn();

sessionService.trackSearch.mockImplementation((params) => ({
complete: completeSearch,
error: jest.fn(),
beforePoll: jest.fn(() => {
return [{ isSearchStored: false }, () => {}];
}),
}));

const req = {
params: {
Expand All @@ -1202,7 +1223,7 @@ describe('SearchInterceptor', () => {
expect(error).toBeCalledTimes(0);
expect(complete).toBeCalledTimes(0);
expect(sessionService.trackSearch).toBeCalledTimes(1);
// expect(untrack).not.toBeCalled();
expect(completeSearch).not.toBeCalled();

const next2 = jest.fn();
const error2 = jest.fn();
Expand All @@ -1218,8 +1239,8 @@ describe('SearchInterceptor', () => {
abortController.abort();

await timeTravel(300);
expect(sessionService.trackSearch).toBeCalledTimes(2);
// expect(untrack).toBeCalledTimes(2);
expect(sessionService.trackSearch).toBeCalledTimes(1);
expect(completeSearch).toBeCalledTimes(1);

expect(next).toBeCalledTimes(2);
expect(error).toBeCalledTimes(0);
Expand All @@ -1233,7 +1254,7 @@ describe('SearchInterceptor', () => {
expect(fetchMock).toBeCalledTimes(2);
});

test.skip('aborting both requests should cancel underlaying search only once', async () => {
test('aborting both requests should cancel underlaying search only once', async () => {
mockFetchImplementation(partialCompleteResponse);
sessionService.isCurrentSession.mockImplementation(
(_sessionId) => _sessionId === sessionId
Expand Down Expand Up @@ -1270,15 +1291,22 @@ describe('SearchInterceptor', () => {
expect(mockCoreSetup.http.delete).toHaveBeenCalledTimes(1);
});

test.skip('aborting both searches should stop searching and clear cache', async () => {
test('aborting both searches should stop searching and clear cache', async () => {
mockFetchImplementation(partialCompleteResponse);
sessionService.isCurrentSession.mockImplementation(
(_sessionId) => _sessionId === sessionId
);
sessionService.getSessionId.mockImplementation(() => sessionId);

// const untrack = jest.fn();
// sessionService.trackSearch.mockImplementation(() => untrack);
const completeSearch = jest.fn();

sessionService.trackSearch.mockImplementation((params) => ({
complete: completeSearch,
error: jest.fn(),
beforePoll: jest.fn(() => {
return [{ isSearchStored: false }, () => {}];
}),
}));

const req = {
params: {
Expand Down
41 changes: 36 additions & 5 deletions src/plugins/data/public/search/search_interceptor/search_interceptor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,18 @@ import {
Subscription,
throwError,
} from 'rxjs';
import { catchError, finalize, map, shareReplay, switchMap, takeUntil, tap } from 'rxjs/operators';
import {
catchError,
filter,
finalize,
map,
shareReplay,
skip,
switchMap,
take,
takeUntil,
tap,
} from 'rxjs/operators';
import { PublicMethodsOf } from '@kbn/utility-types';
import type { HttpSetup, IHttpFetchError } from '@kbn/core-http-browser';

Expand Down Expand Up @@ -57,7 +68,7 @@ import {
TimeoutErrorMode,
SearchSessionIncompleteWarning,
} from '../errors';
import { ISessionService } from '../session';
import { ISessionService, SearchSessionState } from '../session';
import { SearchResponseCache } from './search_response_cache';
import { createRequestHash } from './utils';
import { SearchAbortController } from './search_abort_controller';
Expand Down Expand Up @@ -266,10 +277,27 @@ export class SearchInterceptor {
})
: undefined;

// track if this search's session will be send to background
// if yes, then we don't need to cancel this search when it is aborted
let isSavedToBackground = false;
const savedToBackgroundSub =
this.deps.session.isCurrentSession(sessionId) &&
this.deps.session.state$
.pipe(
skip(1), // ignore any state, we are only interested in transition x -> BackgroundLoading
filter(
(state) =>
this.deps.session.isCurrentSession(sessionId) &&
state === SearchSessionState.BackgroundLoading
),
take(1)
)
.subscribe(() => {
isSavedToBackground = true;
});

const cancel = once(() => {
if (this.deps.session.isCurrentSession(sessionId) && !this.deps.session.isStored()) {
this.deps.http.delete(`/internal/search/${strategy}/${id}`);
}
if (id && !isSavedToBackground) this.deps.http.delete(`/internal/search/${strategy}/${id}`);
});

return pollSearch(search, cancel, {
Expand All @@ -290,6 +318,9 @@ export class SearchInterceptor {
}),
finalize(() => {
searchAbortController.cleanup();
if (savedToBackgroundSub) {
savedToBackgroundSub.unsubscribe();
}
}),
// This observable is cached in the responseCache.
// Using shareReplay makes sure that future subscribers will get the final response
Expand Down

0 comments on commit 01f4b5d

Please sign in to comment.