Skip to content

Commit 5247665

Browse files
Avoid unwanted project persistence. (#1009)
Session storage lasts for the lifetime of the tab, so we need to make sure subsequent migration URLs and iframe configurations work. Motivated by issues with "Open in Classroom" and "Open in Python" on microbit.org.
1 parent 5295490 commit 5247665

File tree

5 files changed

+71
-29
lines changed

5 files changed

+71
-29
lines changed

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/fs/fs.ts

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,15 @@ import {
1010
import { fromByteArray, toByteArray } from "base64-js";
1111
import EventEmitter from "events";
1212
import sortBy from "lodash.sortby";
13+
import { lineNumFromUint8Array } from "../common/text-util";
1314
import { BoardId } from "../device/board-id";
1415
import { FlashDataSource, HexGenerationError } from "../device/device";
1516
import { Logging } from "../logging/logging";
16-
import { Host } from "./host";
17+
import { MicroPythonSource } from "../micropython/micropython";
1718
import { asciiToBytes, generateId } from "./fs-util";
18-
import {
19-
FSStorage,
20-
InMemoryFSStorage,
21-
SessionStorageFSStorage,
22-
SplitStrategyStorage,
23-
} from "./storage";
19+
import { Host } from "./host";
2420
import { PythonProject } from "./initial-project";
25-
import { lineNumFromUint8Array } from "../common/text-util";
26-
import { MicroPythonSource } from "../micropython/micropython";
21+
import { FSStorage } from "./storage";
2722

2823
const commonFsSize = 20 * 1024;
2924

@@ -168,11 +163,7 @@ export class FileSystem extends EventEmitter implements FlashDataSource {
168163
private microPythonSource: MicroPythonSource
169164
) {
170165
super();
171-
this.storage = new SplitStrategyStorage(
172-
new InMemoryFSStorage(undefined),
173-
SessionStorageFSStorage.create(),
174-
logging
175-
);
166+
this.storage = host.createStorage(logging);
176167
this.project = {
177168
files: [],
178169
id: generateId(),
@@ -216,7 +207,7 @@ export class FileSystem extends EventEmitter implements FlashDataSource {
216207
if (!this.initializing) {
217208
this.initializing = (async () => {
218209
this._dirty = await this.storage.isDirty();
219-
if (!(await this.exists(MAIN_FILE))) {
210+
if (await this.host.shouldReinitializeProject(this.storage)) {
220211
// Do this ASAP to unblock the editor.
221212
this.cachedInitialProject = await this.host.createInitialProject();
222213
if (this.cachedInitialProject.projectName) {

src/fs/host.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ import {
1818
projectFilesToBase64,
1919
} from "./initial-project";
2020
import { parseMigrationFromUrl } from "./migration";
21+
import {
22+
FSStorage,
23+
InMemoryFSStorage,
24+
SessionStorageFSStorage,
25+
SplitStrategyStorage,
26+
} from "./storage";
2127

2228
const messages = {
2329
type: "pyeditor",
@@ -30,22 +36,45 @@ const messages = {
3036
};
3137

3238
export interface Host {
39+
createStorage(logging: Logging): FSStorage;
40+
shouldReinitializeProject(storage: FSStorage): Promise<boolean>;
3341
createInitialProject(): Promise<PythonProject>;
3442
notifyReady(fs: FileSystem): void;
3543
}
3644

3745
export class DefaultHost implements Host {
3846
constructor(private url: string = "") {}
3947

40-
async createInitialProject(): Promise<PythonProject> {
48+
createStorage(logging: Logging): FSStorage {
49+
return new SplitStrategyStorage(
50+
new InMemoryFSStorage(undefined),
51+
SessionStorageFSStorage.create(),
52+
logging
53+
);
54+
}
55+
56+
async shouldReinitializeProject(storage: FSStorage): Promise<boolean> {
4157
const migration = parseMigrationFromUrl(this.url);
4258
if (migration) {
43-
return {
59+
return true;
60+
}
61+
return !(await storage.exists(MAIN_FILE));
62+
}
63+
64+
async createInitialProject(): Promise<PythonProject> {
65+
const migrationParseResult = parseMigrationFromUrl(this.url);
66+
if (migrationParseResult) {
67+
const { migration, postMigrationUrl } = migrationParseResult;
68+
const project = {
4469
files: projectFilesToBase64({
4570
[MAIN_FILE]: migration.source,
4671
}),
4772
projectName: migration.meta.name,
4873
};
74+
// Remove the migration information from the URL so that a refresh
75+
// will reload from storage not remigrate.
76+
window.history.replaceState(null, "", postMigrationUrl);
77+
return project;
4978
}
5079
return defaultInitialProject;
5180
}
@@ -58,6 +87,13 @@ export class IframeHost implements Host {
5887
private window: Window,
5988
private debounceDelay: number = 1_000
6089
) {}
90+
createStorage(logging: Logging): FSStorage {
91+
return new InMemoryFSStorage(undefined);
92+
}
93+
async shouldReinitializeProject(storage: FSStorage): Promise<boolean> {
94+
// If there is persistence then it is the embedder's problem.
95+
return true;
96+
}
6197
createInitialProject(): Promise<PythonProject> {
6298
return new Promise((resolve) => {
6399
this.window.addEventListener("load", () =>

src/fs/migration.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,23 @@ import { isMigration, parseMigrationFromUrl } from "./migration";
77

88
// The heart project.
99
export const testMigrationUrl =
10-
"http://localhost:3000/#import:#project:XQAAgACRAAAAAAAAAAA9iImmlGSt1R++5LD+ZJ36cRz46B+lhYtNRoWF0nijpaVyZlK7ACfSpeoQpgfk21st4ty06R4PEOM4sSAXBT95G3en+tghrYmE+YJp6EiYgzA9ThKkyShWq2UdvmCzqxoNfYc1wlmTqlNv/Piaz3WoSe3flvr/ItyLl0aolQlEpv4LA8A=";
10+
// origin needs to match jest's testUrl
11+
"http://localhost/#import:#project:XQAAgACRAAAAAAAAAAA9iImmlGSt1R++5LD+ZJ36cRz46B+lhYtNRoWF0nijpaVyZlK7ACfSpeoQpgfk21st4ty06R4PEOM4sSAXBT95G3en+tghrYmE+YJp6EiYgzA9ThKkyShWq2UdvmCzqxoNfYc1wlmTqlNv/Piaz3WoSe3flvr/ItyLl0aolQlEpv4LA8A=";
1112

1213
describe("parseMigrationFromUrl", () => {
1314
it("parses valid URL", () => {
1415
const migration = parseMigrationFromUrl(testMigrationUrl);
1516
expect(migration).toEqual({
16-
meta: {
17-
cloudId: "microbit.org",
18-
comment: "",
19-
editor: "python",
20-
name: "Hearts",
17+
migration: {
18+
meta: {
19+
cloudId: "microbit.org",
20+
comment: "",
21+
editor: "python",
22+
name: "Hearts",
23+
},
24+
source: `from microbit import *\r\ndisplay.show(Image.HEART)`,
2125
},
22-
source: `from microbit import *\r\ndisplay.show(Image.HEART)`,
26+
postMigrationUrl: "http://localhost/",
2327
});
2428
});
2529
it("undefined for nonsense", () => {

src/fs/migration.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,25 @@ export const isMigration = (v: any): v is Migration =>
2323
typeof v.meta?.name === "string" &&
2424
typeof v.source === "string";
2525

26-
export const parseMigrationFromUrl = (url: string): Migration | undefined => {
27-
const urlPart = url.split("#project:")[1];
26+
interface MigrationParseResult {
27+
migration: Migration;
28+
postMigrationUrl: string;
29+
}
30+
31+
export const parseMigrationFromUrl = (
32+
url: string
33+
): MigrationParseResult | undefined => {
34+
const parts = url.split("#project:");
35+
const urlPart = parts[1];
2836
try {
2937
if (urlPart) {
3038
const bytes = toByteArray(urlPart);
3139
const json = JSON.parse(LZMA.decompress(bytes));
3240
if (isMigration(json)) {
33-
return json;
41+
let postMigrationUrl = parts[0];
42+
// This was previously stripped off by the versioner but for now do it ourselves:
43+
postMigrationUrl = postMigrationUrl.replace(/#import:$/, "");
44+
return { migration: json, postMigrationUrl };
3445
}
3546
}
3647
} catch (e) {

0 commit comments

Comments
 (0)