Skip to content

Commit e9761a7

Browse files
nunomaduronunomaduro
authored
fix(account-client-copy): fixes unhandled promise issue (#1154)
1 parent 49c275d commit e9761a7

File tree

3 files changed

+107
-33
lines changed

3 files changed

+107
-33
lines changed

‎packages/client-account/src/__tests__/integration/account-copy-index.test.ts

Lines changed: 90 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,13 @@
11
import { accountCopyIndex } from '@algolia/client-account';
2+
import { createRetryablePromise } from '@algolia/client-common';
23
import { Rule, Synonym } from '@algolia/client-search';
34

45
import { waitResponses } from '../../../../client-common/src/__tests__/helpers';
56
import { TestSuite } from '../../../../client-common/src/__tests__/TestSuite';
67

78
const testSuite = new TestSuite('account_copy_index');
89

9-
test(testSuite.testName, async () => {
10-
const source = testSuite.makeIndex();
11-
const destination = testSuite
12-
.makeSearchClient('ALGOLIA_APPLICATION_ID_2', 'ALGOLIA_ADMIN_KEY_2')
13-
.initIndex(testSuite.makeIndexName());
14-
15-
await expect(accountCopyIndex(source, source)).rejects.toEqual({
16-
name: 'IndicesInTheSameAppError',
17-
message: 'Indices are in the same application. Use SearchClient.copyIndex instead.',
18-
appId: source.appId,
19-
});
20-
10+
describe(testSuite.testName, () => {
2111
const rule: Rule = {
2212
objectID: 'one',
2313
condition: { anchoring: 'is', pattern: 'pattern' },
@@ -32,29 +22,101 @@ test(testSuite.testName, async () => {
3222

3323
const synomy: Synonym = { objectID: 'one', type: 'synonym', synonyms: ['one', 'two'] };
3424

35-
await waitResponses([
36-
source.saveObject({ objectID: 'one' }),
37-
source.setSettings({ searchableAttributes: ['objectID'] }),
38-
source.saveRule(rule),
39-
source.saveSynonym(synomy),
40-
]);
25+
const setUpSource = async (source: ReturnType<typeof testSuite.makeIndex>) => {
26+
await waitResponses([
27+
source.saveObject({ objectID: 'one' }),
28+
source.setSettings({ searchableAttributes: ['objectID'] }),
29+
source.saveRule(rule),
30+
source.saveSynonym(synomy),
31+
]);
32+
};
33+
34+
const waitForIndexCreated = (destination: ReturnType<typeof testSuite.makeIndex>) =>
35+
createRetryablePromise(async retry => {
36+
const exists = await destination.exists();
37+
38+
return exists ? Promise.resolve() : retry();
39+
});
40+
41+
it('copies indices between accounts', async () => {
42+
const source = testSuite.makeIndex();
43+
const destination = testSuite
44+
.makeSearchClient('ALGOLIA_APPLICATION_ID_2', 'ALGOLIA_ADMIN_KEY_2')
45+
.initIndex(testSuite.makeIndexName());
46+
47+
await setUpSource(source);
48+
49+
await expect(accountCopyIndex(source, source)).rejects.toEqual({
50+
name: 'IndicesInTheSameAppError',
51+
message: 'Indices are in the same application. Use SearchClient.copyIndex instead.',
52+
appId: source.appId,
53+
});
54+
55+
await expect(accountCopyIndex(source, destination).wait()).resolves.toBeUndefined();
56+
57+
await expect(destination.search('')).resolves.toMatchObject({
58+
hits: [{ objectID: 'one' }],
59+
});
4160

42-
await expect(accountCopyIndex(source, destination).wait()).resolves.toBeUndefined();
61+
await expect(destination.getSettings()).resolves.toMatchObject({
62+
searchableAttributes: ['objectID'],
63+
});
4364

44-
await expect(destination.search('')).resolves.toMatchObject({
45-
hits: [{ objectID: 'one' }],
65+
await expect(destination.getRule('one')).resolves.toMatchObject(rule);
66+
67+
await expect(destination.getSynonym('one')).resolves.toEqual(synomy);
68+
69+
await expect(accountCopyIndex(source, destination)).rejects.toEqual({
70+
name: 'DestinationIndiceAlreadyExistsError',
71+
message: 'Destination indice already exists.',
72+
});
4673
});
4774

48-
await expect(destination.getSettings()).resolves.toMatchObject({
49-
searchableAttributes: ['objectID'],
75+
it('it does not need to wait', async () => {
76+
const source = testSuite.makeIndex();
77+
const destination = testSuite
78+
.makeSearchClient('ALGOLIA_APPLICATION_ID_2', 'ALGOLIA_ADMIN_KEY_2')
79+
.initIndex(testSuite.makeIndexName());
80+
81+
await setUpSource(source);
82+
83+
await expect(accountCopyIndex(source, destination)).resolves.toBeUndefined();
84+
85+
await waitForIndexCreated(destination);
86+
await expect(destination.exists()).resolves.toEqual(true);
5087
});
5188

52-
await expect(destination.getRule('one')).resolves.toEqual(rule);
89+
it('it bubbles up errors', async () => {
90+
const source = testSuite.makeIndex();
91+
92+
await setUpSource(source);
93+
94+
const indexName = testSuite.makeIndexName();
95+
96+
const addApiKeyResponse = await testSuite
97+
.makeSearchClient('ALGOLIA_APPLICATION_ID_2', 'ALGOLIA_ADMIN_KEY_2')
98+
.addApiKey(['settings', 'editSettings', 'search'], {
99+
indexes: [indexName],
100+
})
101+
.wait();
102+
103+
const destination = testSuite
104+
.algoliasearch(`${process.env.ALGOLIA_APPLICATION_ID_2}`, addApiKeyResponse.key)
105+
.initIndex(indexName);
106+
107+
await expect(accountCopyIndex(source, destination).wait()).rejects.toMatchObject({
108+
name: 'ApiError',
109+
message: 'Not enough rights to update an object near line:1 column:64',
110+
status: 400,
111+
});
53112

54-
await expect(destination.getSynonym('one')).resolves.toEqual(synomy);
113+
// At this point, we should have created the index. But it should
114+
// be empty because we only have set settings on it.
115+
await waitForIndexCreated(destination);
55116

56-
await expect(accountCopyIndex(source, destination)).rejects.toEqual({
57-
name: 'DestinationIndiceAlreadyExistsError',
58-
message: 'Destination indice already exists.',
117+
await expect(destination.search('')).resolves.toMatchObject({
118+
nbHits: 0,
119+
hits: [],
120+
});
59121
});
60122
});

‎packages/client-account/src/methods/accountCopyIndex.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,22 @@ export const accountCopyIndex = (
5555
// eslint-disable-next-line functional/immutable-data
5656
batch: objects => responses.push(saveObjects(destination)(objects, requestOptions)),
5757
})
58-
)
59-
.then(() => Promise.resolve());
58+
);
59+
60+
return createWaitablePromise(
61+
/**
62+
* The original promise will return an array of async responses, now
63+
* we need to resolve that array of async responses using a
64+
* `Promise.all`, and then resolve `void` for the end-user.
65+
*/
66+
promise.then(() => Promise.all(responses)).then(() => undefined),
6067

61-
return createWaitablePromise(promise, () =>
62-
Promise.all(responses.map(response => response.wait()))
68+
/**
69+
* Next, if the end-user calls the `wait` method, we need to also call
70+
* the `wait` method on each element of of async responses.
71+
*/
72+
(_response, waitRequestOptions) => {
73+
return Promise.all(responses.map(response => response.wait(waitRequestOptions)));
74+
}
6375
);
6476
};

‎packages/client-search/src/__tests__/integration/replacing.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ test(testSuite.testName, async () => {
7474
status: 404,
7575
});
7676

77-
await expect(index.getRule('two')).resolves.toEqual(rule2);
77+
await expect(index.getRule('two')).resolves.toMatchObject(rule2);
7878
await expect(index.getRule('one')).rejects.toMatchObject({
7979
name: 'ApiError',
8080
message: 'ObjectID does not exist',

0 commit comments

Comments
 (0)