From a9251835095906942247322762c779276a58a19c Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Tue, 11 Apr 2023 10:06:12 +0200 Subject: [PATCH] fix: try fetch the sketch by path if not in the cache The sketch cache might be empty, when trying to generate the secrets include in the main sketch file from the `secrets` property. Closes #1999 Signed-off-by: Akos Kitta --- .../src/browser/create/create-api.ts | 61 ++++++++++++++++++- .../cloud-sketchbook/cloud-sketch-cache.ts | 12 +++- .../src/test/browser/create-api.test.ts | 34 +++++++++++ 3 files changed, 102 insertions(+), 5 deletions(-) diff --git a/arduino-ide-extension/src/browser/create/create-api.ts b/arduino-ide-extension/src/browser/create/create-api.ts index 72c27cbb0..b2aac2b02 100644 --- a/arduino-ide-extension/src/browser/create/create-api.ts +++ b/arduino-ide-extension/src/browser/create/create-api.ts @@ -57,6 +57,30 @@ export class CreateApi { return result; } + /** + * `sketchPath` is not the POSIX path but the path with the user UUID, username, etc. + * See [Create.Resource#path](./typings.ts). If `cache` is `true` and a sketch exists with the path, + * the cache will be updated with the new state of the sketch. + */ + // TODO: no nulls in API + async sketchByPath( + sketchPath: string, + cache = false + ): Promise { + const url = new URL(`${this.domain()}/sketches/byPath/${sketchPath}`); + const headers = await this.headers(); + const sketch = await this.run(url, { + method: 'GET', + headers, + }); + if (sketch && cache) { + this.sketchCache.addSketch(sketch); + const posixPath = createPaths.toPosixPath(sketch.path); + this.sketchCache.purgeByPath(posixPath); + } + return sketch; + } + async sketches(limit = 50): Promise { const url = new URL(`${this.domain()}/sketches`); url.searchParams.set('user_id', 'me'); @@ -86,7 +110,11 @@ export class CreateApi { async createSketch( posixPath: string, - contentProvider: MaybePromise = this.sketchesService.defaultInoContent() + contentProvider: MaybePromise = this.sketchesService.defaultInoContent(), + payloadOverride: Record< + string, + string | boolean | number | Record + > = {} ): Promise { const url = new URL(`${this.domain()}/sketches`); const [headers, content] = await Promise.all([ @@ -97,6 +125,7 @@ export class CreateApi { ino: btoa(content), path: posixPath, user_id: 'me', + ...payloadOverride, }; const init = { method: 'PUT', @@ -212,7 +241,17 @@ export class CreateApi { return data; } - const sketch = this.sketchCache.getSketch(createPaths.parentPosix(path)); + const posixPath = createPaths.parentPosix(path); + let sketch = this.sketchCache.getSketch(posixPath); + // Workaround for https://github.com/arduino/arduino-ide/issues/1999. + if (!sketch) { + // Convert the ordinary sketch POSIX path to the Create path. + // For example, `/sketch_apr6a` will be transformed to `8a694e4b83878cc53472bd75ee928053:kittaakos/sketches_v2/sketch_apr6a`. + const createPathPrefix = this.sketchCache.createPathPrefix; + if (createPathPrefix) { + sketch = await this.sketchByPath(createPathPrefix + posixPath, true); + } + } if ( sketch && @@ -448,13 +487,18 @@ export class CreateApi { await this.run(url, init, ResponseResultProvider.NOOP); } + private fetchCounter = 0; private async run( requestInfo: URL, init: RequestInit | undefined, resultProvider: ResponseResultProvider = ResponseResultProvider.JSON ): Promise { - console.debug(`HTTP ${init?.method}: ${requestInfo.toString()}`); + const fetchCount = `[${++this.fetchCounter}]`; + const fetchStart = performance.now(); + const method = init?.method ? `${init.method}: ` : ''; + const url = requestInfo.toString(); const response = await fetch(requestInfo.toString(), init); + const fetchEnd = performance.now(); if (!response.ok) { let details: string | undefined = undefined; try { @@ -465,7 +509,18 @@ export class CreateApi { const { statusText, status } = response; throw new CreateError(statusText, status, details); } + const parseStart = performance.now(); const result = await resultProvider(response); + const parseEnd = performance.now(); + console.debug( + `HTTP ${fetchCount} ${method} ${url} [fetch: ${( + fetchEnd - fetchStart + ).toFixed(2)} ms, parse: ${(parseEnd - parseStart).toFixed( + 2 + )} ms] body: ${ + typeof result === 'string' ? result : JSON.stringify(result) + }` + ); return result; } diff --git a/arduino-ide-extension/src/browser/widgets/cloud-sketchbook/cloud-sketch-cache.ts b/arduino-ide-extension/src/browser/widgets/cloud-sketchbook/cloud-sketch-cache.ts index 21469d2de..0b0fc024a 100644 --- a/arduino-ide-extension/src/browser/widgets/cloud-sketchbook/cloud-sketch-cache.ts +++ b/arduino-ide-extension/src/browser/widgets/cloud-sketchbook/cloud-sketch-cache.ts @@ -1,12 +1,13 @@ import { FileStat } from '@theia/filesystem/lib/common/files'; import { injectable } from '@theia/core/shared/inversify'; -import { toPosixPath } from '../../create/create-paths'; +import { splitSketchPath } from '../../create/create-paths'; import { Create } from '../../create/typings'; @injectable() export class SketchCache { sketches: Record = {}; fileStats: Record = {}; + private _createPathPrefix: string | undefined; init(): void { // reset the data @@ -32,7 +33,10 @@ export class SketchCache { addSketch(sketch: Create.Sketch): void { const { path } = sketch; - const posixPath = toPosixPath(path); + const [pathPrefix, posixPath] = splitSketchPath(path); + if (pathPrefix !== this._createPathPrefix) { + this._createPathPrefix = pathPrefix; + } this.sketches[posixPath] = sketch; } @@ -40,6 +44,10 @@ export class SketchCache { return this.sketches[path] || null; } + get createPathPrefix(): string | undefined { + return this._createPathPrefix; + } + toString(): string { return JSON.stringify({ sketches: this.sketches, diff --git a/arduino-ide-extension/src/test/browser/create-api.test.ts b/arduino-ide-extension/src/test/browser/create-api.test.ts index a623854c6..a18609cb1 100644 --- a/arduino-ide-extension/src/test/browser/create-api.test.ts +++ b/arduino-ide-extension/src/test/browser/create-api.test.ts @@ -1,6 +1,7 @@ import { Container, ContainerModule } from '@theia/core/shared/inversify'; import { assert, expect } from 'chai'; import fetch from 'cross-fetch'; +import { posix } from 'path'; import { v4 } from 'uuid'; import { ArduinoPreferences } from '../../browser/arduino-preferences'; import { AuthenticationClientService } from '../../browser/auth/authentication-client-service'; @@ -251,6 +252,39 @@ describe('create-api', () => { expect(sketch).to.be.undefined; }); }); + + it("should fetch the sketch when transforming the 'secrets' into '#include' and the sketch is not in the cache", async () => { + const name = v4(); + const posixPath = toPosix(name); + const newSketch = await createApi.createSketch( + posixPath, + 'void setup(){} void loop(){}', + { + secrets: { + data: [ + { + name: 'SECRET_THING', + value: '❤︎', + }, + ], + }, + } + ); + expect(newSketch).to.be.not.undefined; + expect(newSketch.secrets).to.be.not.undefined; + expect(Array.isArray(newSketch.secrets)).to.be.true; + expect(newSketch.secrets?.length).to.be.equal(1); + expect(newSketch.secrets?.[0]).to.be.deep.equal({ + name: 'SECRET_THING', + value: '❤︎', + }); + createApi.sketchCache.init(); // invalidate the cache + const content = await createApi.readFile( + posix.join(posixPath, `${name}.ino`) + ); + expect(content.includes(`#include "${Create.arduino_secrets_file}"`)).to.be + .true; + }); }); // Using environment variables is recommended for testing but you can modify the module too.