Skip to content

[server] finished refactoring BoardServerStore.upsert() #5327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[server] finished refactoring BoardServerStore.upsert() #5327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 22 additions & 40 deletions packages/board-server/src/server.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,21 @@ import { afterEach, before, suite, test } from "node:test";
import request from "supertest";

import { createServer, createServerConfig } from "./server.js";
import type { BoardServerStore } from "./server/store.js";
import type { BoardServerStore, StorageBoard } from "./server/store.js";

suite("Board Server integration test", () => {
const user = { username: "test-user", apiKey: "test-api-key" };

let server: Express;
let store: BoardServerStore;
const defaultBoard: Readonly<Partial<StorageBoard>> = {
name: "test-board",
owner: user.username,
graph: {
nodes: [{ type: "input", id: "input" }],
edges: [],
},
};

before(async () => {
process.env.STORAGE_BUCKET = "test-bucket";
Expand Down Expand Up @@ -75,7 +83,7 @@ suite("Board Server integration test", () => {
});

test("GET /boards/@:user/:name", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server).get(
`/boards/@${user.username}/test-board?API_KEY=${user.apiKey}`
Expand All @@ -85,7 +93,7 @@ suite("Board Server integration test", () => {
});

test("GET /:name", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server).get(
`/boards/test-board?API_KEY=${user.apiKey}`
Expand All @@ -95,7 +103,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name -> updates", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server)
.post(`/boards/@${user.username}/test-board?API_KEY=${user.apiKey}`)
Expand All @@ -106,7 +114,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/:name -> updates", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server)
.post(`/boards/test-board?API_KEY=${user.apiKey}`)
Expand All @@ -117,7 +125,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name -> deletes", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server)
.post(`/boards/@${user.username}/test-board?API_KEY=${user.apiKey}`)
Expand All @@ -130,7 +138,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/:name -> deletes", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server)
.post(`/boards/test-board?API_KEY=${user.apiKey}`)
Expand All @@ -143,20 +151,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name.api/invoke", async () => {
await store.createBoard(user.username, "test-board.json");
await store.updateBoard({
name: "test-board.json",
owner: user.username,
displayName: "",
description: "",
tags: [],
thumbnail: "",
// TODO make this a real board that runs
graph: {
nodes: [{ type: "input", id: "input" }],
edges: [],
},
});
await store.upsertBoard({...defaultBoard, name: 'test-board.json'});

const response = await request(server)
.post(`/boards/@${user.username}/test-board.api/invoke`)
Expand All @@ -166,20 +161,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name/invoke", async () => {
await store.createBoard(user.username, "test-board");
await store.updateBoard({
name: "test-board",
owner: user.username,
displayName: "",
description: "",
tags: [],
thumbnail: "",
// TODO make this a real board that runs
graph: {
nodes: [{ type: "input", id: "input" }],
edges: [],
},
});
await store.upsertBoard(defaultBoard);

const response = await request(server)
.post(`/boards/@${user.username}/test-board/invoke`)
Expand All @@ -189,7 +171,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name.api/describe", async () => {
await store.createBoard(user.username, "test-board.json");
await store.upsertBoard({...defaultBoard, name: 'test-board.json'});
const path = `@${user.username}/test-board.api`;

const response = await request(server)
Expand All @@ -200,7 +182,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name/describe", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);
const path = `@${user.username}/test-board`;

const response = await request(server)
Expand All @@ -211,7 +193,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/:name/describe", async () => {
await store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server)
.post(`/boards/test-board/describe?API_KEY=${user.apiKey}`)
Expand All @@ -221,7 +203,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name.api/run", async () => {
store.createBoard(user.username, "test-board.json");
await store.upsertBoard({...defaultBoard, name: 'test-board.json'});

const response = await request(server)
.post(`/boards/@${user.username}/test-board.api/run`)
Expand All @@ -231,7 +213,7 @@ suite("Board Server integration test", () => {
});

test("POST /boards/@:user/:name/run", async () => {
store.createBoard(user.username, "test-board");
await store.upsertBoard(defaultBoard);

const response = await request(server)
.post(`/boards/@${user.username}/test-board/run`)
Expand Down
13 changes: 5 additions & 8 deletions packages/board-server/src/server/storage-providers/firestore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ export class FirestoreStorageProvider
});
}

async updateBoard(board: Readonly<Partial<StorageBoard>>): Promise<void> {
async upsertBoard(board: Readonly<Partial<StorageBoard>>): Promise<StorageBoard> {
const name = board.name || crypto.randomUUID();
const updatedBoard: Partial<StorageBoard> = {...board, name};

if (!board.owner) {
throw new InvalidRequestError("Firestore requires an owner set");
}
Expand All @@ -158,15 +161,9 @@ export class FirestoreStorageProvider
tags: board.tags || [],
graph: JSON.stringify(board.graph || {}),
});
}

async upsertBoard(board: Readonly<Partial<StorageBoard>>): Promise<StorageBoard> {
const name = board.name || crypto.randomUUID();
const updatedBoard: Partial<StorageBoard> = {...board, name};
await this.updateBoard(updatedBoard);
const result = await this.loadBoard({name, owner: updatedBoard.owner});
if (!result) {
throw new Error(`Failed to create the board ${updatedBoard.name}`);
throw new Error(`upsertBoard(${updatedBoard.name}) failed`);
}
return result;
}
Expand Down
19 changes: 8 additions & 11 deletions packages/board-server/src/server/storage-providers/inmemory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ suite("In-memory storage provider", () => {
test("create board", async () => {
assert.equal(await provider.loadBoard({ name: "test-board" }), null);

provider.createBoard("user", "test-board");
provider.upsertBoard({owner: "user", name: "test-board"});

assert.deepEqual(await provider.loadBoard({ name: "test-board" }), {
name: "test-board",
owner: "user",
displayName: "test-board",
displayName: "",
description: "",
tags: [],
thumbnail: "",
Expand Down Expand Up @@ -65,7 +65,7 @@ suite("In-memory storage provider", () => {
};

// Technically, it's not necessary to create the board first
provider.updateBoard(updatedBoard);
provider.upsertBoard(updatedBoard);

assert.deepEqual(
await provider.loadBoard({ name: "test-board" }),
Expand Down Expand Up @@ -104,7 +104,7 @@ suite("In-memory storage provider", () => {
graph: GRAPH,
};

await provider.updateBoard(board);
await provider.upsertBoard(board);

assert.deepEqual(await provider.loadBoard({ name: "test-board" }), board);
assert.equal(await provider.loadBoard({ name: "other-board" }), null);
Expand Down Expand Up @@ -139,15 +139,12 @@ suite("In-memory storage provider", () => {
graph: GRAPH,
};

provider.updateBoard(ownedBoard);
provider.updateBoard(publishedBoard);
provider.updateBoard(privateBoard);
provider.upsertBoard(ownedBoard);
provider.upsertBoard(publishedBoard);
provider.upsertBoard(privateBoard);

const boards = await provider.listBoards("me");

assert(boards.includes(ownedBoard));
assert(boards.includes(publishedBoard));
assert(!boards.includes(privateBoard));
assert.deepEqual(boards.map((board => board.name)), ['ownedBoard', 'publishedBoard']);
});
});

Expand Down
26 changes: 9 additions & 17 deletions packages/board-server/src/server/storage-providers/inmemory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,16 @@ export class InMemoryStorageProvider implements BoardServerStore {
});
}

async createBoard(userId: string, name: string): Promise<void> {
this.#boards[name] = {
name,
owner: userId,
displayName: name,
description: "",
tags: [],
thumbnail: "",
graph: blank(),
async upsertBoard(board: Readonly<Partial<StorageBoard>>): Promise<StorageBoard> {
const updatedBoard: StorageBoard = {
name: board.name || crypto.randomUUID(),
owner: board.owner || "",
displayName: board.displayName || "",
description: board.description || "",
tags: board.tags || [],
thumbnail: board.thumbnail || "",
graph: board.graph || blank(),
};
}

async updateBoard(board: StorageBoard): Promise<void> {
this.#boards[board.name] = board;
}

async upsertBoard(board: Readonly<StorageBoard>): Promise<StorageBoard> {
const updatedBoard: StorageBoard = {...board, name: board.name || crypto.randomUUID()};
this.#boards[updatedBoard.name] = updatedBoard;
return updatedBoard;
}
Expand Down
19 changes: 1 addition & 18 deletions packages/board-server/src/server/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,28 +68,11 @@ export interface BoardServerStore {
/** List all boards visible to the given user. */
listBoards(userId: string): Promise<StorageBoard[]>;

/**
* Create a blank board with no graph.
*
* TODO This shouldn't really be necessary, we can just use "update"
*
* @deprecated migrate to upsertBoard() API.
*/
createBoard(userId: string, name: string): Promise<void>;

/**
* Updates the given board. Creates if it doesn't exist.
*
* @deprecated migrate to upsertBoard() API.
*/
updateBoard(board: StorageBoard): Promise<void>;


/**
* Creates or inserts the given board.
*
*/
upsertBoard(board: Readonly<StorageBoard>): Promise<StorageBoard>;
upsertBoard(board: Readonly<Partial<StorageBoard>>): Promise<StorageBoard>;

/** Deletes a board by name */
deleteBoard(userId: string, boardName: string): Promise<void>;
Expand Down
16 changes: 8 additions & 8 deletions packages/board-server/tests/firestore/firestore-storage-provider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ suite("Firestore storage provider", () => {
};

// Technically, it's not necessary to create the board first
await provider.updateBoard(updatedBoard);
await provider.upsertBoard(updatedBoard);

assertBoard(updatedBoard);
assertGraph(updatedBoard);
Expand Down Expand Up @@ -115,10 +115,10 @@ suite("Firestore storage provider", () => {
graph: GRAPH,
};

await provider.updateBoard(ownedBoard);
await provider.updateBoard(publishedBoard);
await provider.updateBoard(unlistedBoard);
await provider.updateBoard(privateBoard);
await provider.upsertBoard(ownedBoard);
await provider.upsertBoard(publishedBoard);
await provider.upsertBoard(unlistedBoard);
await provider.upsertBoard(privateBoard);

const ownedBoardResult = await provider.loadBoard({
name: "owned-board",
Expand Down Expand Up @@ -177,9 +177,9 @@ suite("Firestore storage provider", () => {
graph: GRAPH,
};

await provider.updateBoard(ownedBoard);
await provider.updateBoard(publishedBoard);
await provider.updateBoard(privateBoard);
await provider.upsertBoard(ownedBoard);
await provider.upsertBoard(publishedBoard);
await provider.upsertBoard(privateBoard);

const boards = await provider.listBoards("me");
assert.equal(boards.length, 2);
Expand Down