Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 097c92d

Browse files
Akos Kittakittaakos
Akos Kitta
authored andcommitted
fix: reduced unnecessary GET /sketches request
Ref: #1849 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent b451e2d commit 097c92d

File tree

2 files changed

+107
-21
lines changed

2 files changed

+107
-21
lines changed

‎arduino-ide-extension/src/browser/create/create-api.ts‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,26 +86,25 @@ export class CreateApi {
8686
url.searchParams.set('user_id', 'me');
8787
url.searchParams.set('limit', limit.toString());
8888
const headers = await this.headers();
89-
const result: { sketches: Create.Sketch[] } = { sketches: [] };
90-
91-
let partialSketches: Create.Sketch[] = [];
89+
const allSketches: Create.Sketch[] = [];
9290
let currentOffset = 0;
93-
do {
91+
while(true) {
9492
url.searchParams.set('offset', currentOffset.toString());
95-
partialSketches = (
96-
await this.run<{ sketches: Create.Sketch[] }>(url, {
97-
method: 'GET',
98-
headers,
99-
})
100-
).sketches;
101-
if (partialSketches.length !== 0) {
102-
result.sketches = result.sketches.concat(partialSketches);
93+
const { sketches } = await this.run<{ sketches: Create.Sketch[] }>(url, {
94+
method: 'GET',
95+
headers,
96+
});
97+
allSketches.push(...sketches);
98+
if (sketches.length < limit) {
99+
break;
103100
}
104-
currentOffset = currentOffset + limit;
105-
} while (partialSketches.length !== 0);
106-
107-
result.sketches.forEach((sketch) => this.sketchCache.addSketch(sketch));
108-
return result.sketches;
101+
currentOffset += limit;
102+
// The create API doc show that there is `next` and `prev` pages, but it does not work
103+
// https://api2.arduino.cc/create/docs#!/sketches95v2/sketches_v2_search
104+
// IF sketchCount mod limit === 0, an extra fetch must happen to detect the end of the pagination.
105+
}
106+
allSketches.forEach((sketch) => this.sketchCache.addSketch(sketch));
107+
return allSketches;
109108
}
110109

111110
async createSketch(

‎arduino-ide-extension/src/test/browser/create-api.test.ts‎

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { Container, ContainerModule } from '@theia/core/shared/inversify';
1+
import {
2+
Container,
3+
ContainerModule,
4+
injectable,
5+
} from '@theia/core/shared/inversify';
26
import { assert, expect } from 'chai';
37
import fetch from 'cross-fetch';
48
import { posix } from 'path';
@@ -19,13 +23,14 @@ import queryString = require('query-string');
1923
const timeout = 60 * 1_000;
2024

2125
describe('create-api', () => {
22-
let createApi: CreateApi;
26+
let createApi: TestCreateApi;
2327

2428
before(async function () {
2529
this.timeout(timeout);
2630
try {
2731
const accessToken = await login();
28-
createApi = createContainer(accessToken).get<CreateApi>(CreateApi);
32+
createApi =
33+
createContainer(accessToken).get<TestCreateApi>(TestCreateApi);
2934
} catch (err) {
3035
if (err instanceof LoginFailed) {
3136
return this.skip();
@@ -43,7 +48,7 @@ describe('create-api', () => {
4348
const container = new Container({ defaultScope: 'Singleton' });
4449
container.load(
4550
new ContainerModule((bind) => {
46-
bind(CreateApi).toSelf().inSingletonScope();
51+
bind(TestCreateApi).toSelf().inSingletonScope();
4752
bind(SketchCache).toSelf().inSingletonScope();
4853
bind(AuthenticationClientService).toConstantValue(<
4954
AuthenticationClientService
@@ -224,6 +229,47 @@ describe('create-api', () => {
224229
expect(findByName(otherName, sketches)).to.be.not.undefined;
225230
});
226231

232+
[
233+
[-1, 1],
234+
[0, 2],
235+
[1, 2],
236+
].forEach(([diff, expected]) =>
237+
it(`should not run unnecessary fetches when retrieving all sketches (sketch count ${
238+
diff < 0 ? '<' : diff > 0 ? '>' : '='
239+
} limit)`, async () => {
240+
const content = 'void setup(){} void loop(){}';
241+
const maxLimit = 50; // https://github.com/arduino/arduino-ide/pull/875
242+
const sketchCount = maxLimit + diff;
243+
const sketchNames = [...Array(sketchCount).keys()].map(() => v4());
244+
245+
await sketchNames
246+
.map((name) => createApi.createSketch(toPosix(name), content))
247+
.reduce(async (acc, curr) => {
248+
await acc;
249+
return curr;
250+
}, Promise.resolve() as Promise<unknown>);
251+
252+
createApi.resetRequestRecording();
253+
const sketches = await createApi.sketches();
254+
const allRequests = createApi.requestRecording.slice();
255+
256+
expect(sketches.length).to.be.equal(sketchCount);
257+
sketchNames.forEach(
258+
(name) => expect(findByName(name, sketches)).to.be.not.undefined
259+
);
260+
261+
expect(allRequests.length).to.be.equal(expected);
262+
const getSketchesRequests = allRequests.filter(
263+
(description) =>
264+
description.method === 'GET' &&
265+
description.pathname === '/create/v2/sketches' &&
266+
description.query &&
267+
description.query.includes(`limit=${maxLimit}`)
268+
);
269+
expect(getSketchesRequests.length).to.be.equal(expected);
270+
})
271+
);
272+
227273
['.', '-', '_'].map((char) => {
228274
it(`should create a new sketch with '${char}' in the sketch folder name although it's disallowed from the Create Editor`, async () => {
229275
const name = `sketch${char}`;
@@ -332,3 +378,44 @@ class LoginFailed extends Error {
332378
Object.setPrototypeOf(this, LoginFailed.prototype);
333379
}
334380
}
381+
382+
@injectable()
383+
class TestCreateApi extends CreateApi {
384+
private _recording: RequestDescription[] = [];
385+
386+
constructor() {
387+
super();
388+
const originalRun = this['run'];
389+
this['run'] = (url, init, resultProvider) => {
390+
this._recording.push(createRequestDescription(url, init));
391+
return originalRun.bind(this)(url, init, resultProvider);
392+
};
393+
}
394+
395+
resetRequestRecording(): void {
396+
this._recording = [];
397+
}
398+
399+
get requestRecording(): RequestDescription[] {
400+
return this._recording;
401+
}
402+
}
403+
404+
interface RequestDescription {
405+
readonly origin: string;
406+
readonly pathname: string;
407+
readonly query?: string;
408+
409+
readonly method?: string | undefined;
410+
readonly serializedBody?: string | undefined;
411+
}
412+
413+
function createRequestDescription(
414+
url: URL,
415+
init?: RequestInit | undefined
416+
): RequestDescription {
417+
const { origin, pathname, search: query } = url;
418+
const method = init?.method;
419+
const serializedBody = init?.body?.toString();
420+
return { origin, pathname, query, method, serializedBody };
421+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /