You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/20 00:08:15 UTC
(texera) branch gh-readonly-queue/main/pr-7791-9c5aeccdfd6d77712a04623458c6e3113cd60202 created (now 68c5774a82)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7791-9c5aeccdfd6d77712a04623458c6e3113cd60202
in repository https://gitbox.apache.org/repos/asf/texera.git
at 68c5774a82 refactor(python-notebook-migration): key the notebook mapping to the workflow's own version instead of a hardcoded vid=1 (#7791)
This branch includes the following new commits:
new 68c5774a82 refactor(python-notebook-migration): key the notebook mapping to the workflow's own version instead of a hardcoded vid=1 (#7791)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(texera) 01/01: refactor(python-notebook-migration): key the notebook mapping to the workflow's own version instead of a hardcoded vid=1 (#7791)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7791-9c5aeccdfd6d77712a04623458c6e3113cd60202
in repository https://gitbox.apache.org/repos/asf/texera.git
commit 68c5774a827f872c99f22c1b64060abee835cde1
Author: Ryan Zhang <97...@users.noreply.github.com>
AuthorDate: Thu Aug 20 00:07:55 2026 +0000
refactor(python-notebook-migration): key the notebook mapping to the workflow's own version instead of a hardcoded vid=1 (#7791)
### What changes were proposed in this PR?
This PR resolves the real version ID of a workflow instead of the
constant, and makes the fetch version-agnostic so a mapping keeps
reattaching after the workflow's version advances. `notebook.wid` is
UNIQUE (one notebook per workflow) and the fetch join is on `wid + nid`,
so keying on `wid` alone is sufficient and correct.
**`NotebookMigrationResource` (store resolves the version, fetch drops
the vid filter)**
- Store no longer reads a vid from the request. It resolves the
mapping's vid server-side as `MAX(workflow_version.vid)` for the wid,
anchoring the mapping to the workflow's own latest version rather than a
client-supplied value. If the workflow has no version to anchor to, it
returns a 400 before any insert instead of a 500 from the FK.
- Fetch no longer reads a vid or filters the query by it, so the stored
mapping still resolves after the workflow advances to a newer version.
- Adds a future-work note at the fetch query: supporting one notebook
per workflow version would mean dropping the `notebook.wid` UNIQUE
constraint and re-adding a vid filter (falling back to the latest),
since without it a multi-notebook workflow would silently return only
the newest.
**`NotebookMigrationService` (frontend store)**
- `storeNotebookAndMapping` drops its `vid` parameter and stops sending
`vid` in the request body. The version is now resolved by the backend.
**`JupyterPanelService` (frontend fetch)**
- `fetchNotebookAndMapping` drops its `vId` parameter and the `vid`
field from the request body, along with the stale "Future work: add
dynamic fetching of current workflow vId" comment. The panel reattaches
by wid.
### Any related issues, documentation, discussions?
Closes #7636
Parent issue #4301
### How was this PR tested?
Added tests in
- `NotebookMigrationResourceSpec.scala`
- `notebook-migration.service.spec.ts`
- `jupyter-panel.service.spec.ts`
Also manually verified on local development environment.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
---
.../jupyter-panel/jupyter-panel.service.spec.ts | 47 +++-----------
.../service/jupyter-panel/jupyter-panel.service.ts | 21 +-----
.../notebook-migration.service.spec.ts | 4 +-
.../notebook-migration.service.ts | 6 +-
.../resource/NotebookMigrationResource.scala | 29 +++++++--
.../resource/NotebookMigrationResourceSpec.scala | 74 ++++++++++++++++------
6 files changed, 91 insertions(+), 90 deletions(-)
diff --git a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts
index 256e8f2cba..3abaddd342 100644
--- a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts
+++ b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts
@@ -123,7 +123,7 @@ describe("JupyterPanelService", () => {
service.jupyterNotebookPanelVisible$.subscribe(v => (state = v));
- service.openPanel("JupyterNotebookPanel");
+ (service as any).jupyterNotebookPanelVisible.next(true);
expect(state).toBe(true);
service.deleteJupyterNotebook();
@@ -221,38 +221,14 @@ describe("JupyterPanelService", () => {
expect(state).toBe(true);
});
- // openPanel
- it("should open panel only for correct name", () => {
- let state: boolean | null = false;
-
- service.jupyterNotebookPanelVisible$.subscribe(v => (state = v));
-
- service.openPanel("WrongPanel");
- expect(state).toBe(false);
-
- service.openPanel("JupyterNotebookPanel");
- expect(state).toBe(true);
- });
-
- it("openPanel flags jupyterNotebookExists$ so the toolbar expand button appears after an in-place import", () => {
- const states: boolean[] = [];
- service.jupyterNotebookExists$.subscribe(v => states.push(v));
- expect(states.at(-1)).toBe(false);
-
- // Wrong panel name does not flip the flag.
- service.openPanel("WrongPanel");
- expect(states.at(-1)).toBe(false);
-
- // Opening the jupyter panel records that the workflow now has a notebook.
- service.openPanel("JupyterNotebookPanel");
- expect(states.at(-1)).toBe(true);
- });
-
// HTTP fetchNotebookAndMapping
it("should return 0 when exists=false", async () => {
- const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1, 1));
+ const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1));
const req = httpMock.expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"));
+ // The fetch keys on wid only; no vid is sent (the backend ignores version).
+ expect(req.request.body.wid).toBe(1);
+ expect(req.request.body.vid).toBeUndefined();
req.flush({ exists: false });
expect(await resultPromise).toBe(0);
@@ -271,7 +247,7 @@ describe("JupyterPanelService", () => {
const mapping = { cell_to_operator: { cell1: ["A"] }, operator_to_cell: {} };
const notebook = { cells: [] };
- const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1, 1));
+ const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1));
httpMock
.expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"))
.flush({ exists: true, mapping, notebook });
@@ -289,7 +265,7 @@ describe("JupyterPanelService", () => {
mockNotebook.sendNotebookToJupyter = vi.fn().mockResolvedValue(1);
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
- const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1, 1));
+ const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1));
httpMock
.expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"))
.flush("migration service down", { status: 500, statusText: "Server Error" });
@@ -307,7 +283,7 @@ describe("JupyterPanelService", () => {
const mapping = { cell_to_operator: {}, operator_to_cell: {} };
const notebook = { cells: [] };
- const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(2, 1));
+ const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(2));
httpMock
.expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"))
.flush({ exists: true, mapping, notebook });
@@ -716,13 +692,6 @@ describe("JupyterPanelService", () => {
expect(mockWorkflow.workflowMetaDataChanged).not.toHaveBeenCalled();
});
- it("openPanel does not flip the visibility stream", () => {
- let state: boolean | null = false;
- service.jupyterNotebookPanelVisible$.subscribe(v => (state = v));
- service.openPanel("JupyterNotebookPanel");
- expect(state).toBe(false);
- });
-
it("deleteJupyterNotebook does not call the backend or delete the mapping when disabled", () => {
// When the feature is disabled the method returns early, so neither the
// backend delete nor the local mapping drop should run.
diff --git a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts
index 2a1681155d..8f4624867d 100644
--- a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts
+++ b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts
@@ -125,16 +125,12 @@ export class JupyterPanelService {
});
}
- private fetchNotebookAndMapping(
- workflowID: number | undefined = this.workflowActionService.getWorkflow().wid,
- vId: number = 1
- ) {
- // Fetch mapping and notebook from migration database if exists for wid
+ private fetchNotebookAndMapping(workflowID: number | undefined = this.workflowActionService.getWorkflow().wid) {
+ // Fetch mapping and notebook from migration database if exists for wid.
const dbAPIUrl = `${AppSettings.getApiEndpoint()}/notebook-migration/fetch-notebook-and-mapping`;
const headers = new HttpHeaders({ "Content-Type": "application/json" });
const payload = {
wid: workflowID,
- vid: vId, // Future work: add dynamic fetching of current workflow vId
};
return this.http.post(dbAPIUrl, payload, { headers }).pipe(
@@ -224,19 +220,6 @@ export class JupyterPanelService {
return this.notebookMigrationService.getJupyterIframeURL(this.currentNotebookFileName());
}
- // Open the Jupyter Notebook panel
- public openPanel(panelName: string): void {
- if (!this.enabled) return;
- if (panelName === "JupyterNotebookPanel") {
- this.jupyterNotebookPanelVisible.next(true);
- // Opening the panel means the current workflow has an associated notebook, so
- // surface the toolbar "expand" button (jupyterNotebookExists$) right away. Needed
- // after an in-place import where the wid does not change and init() does not re-run
- // to detect the notebook.
- this.jupyterNotebookExists.next(true);
- }
- }
-
// Delete the current workflow's stored notebook from the migration database and its file
// from the Jupyter pod, then hide the panel and clear all local notebook state.
public deleteJupyterNotebook(): void {
diff --git a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts
index 26fa39ee4a..7ffa452a48 100644
--- a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts
+++ b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts
@@ -264,14 +264,14 @@ describe("NotebookMigrationService", () => {
});
// storeNotebookAndMapping
- it("should call storeNotebookAndMapping API with the default vid", () => {
+ it("should call storeNotebookAndMapping API without a vid (resolved server-side)", () => {
service.storeNotebookAndMapping(1, {}, {}).subscribe();
const req = httpMock.expectOne(req => req.url.includes("/notebook-migration/store-notebook-and-mapping"));
expect(req.request.method).toBe("POST");
expect(req.request.body.wid).toBe(1);
- expect(req.request.body.vid).toBe(1);
+ expect(req.request.body.vid).toBeUndefined();
req.flush({ success: true, message: "stored" });
});
diff --git a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts
index 1cb5b92b8e..ed3ffd5a6e 100644
--- a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts
+++ b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts
@@ -228,8 +228,7 @@ export class NotebookMigrationService {
public storeNotebookAndMapping(
wid: number | undefined,
mappingContent: any,
- notebookContent: any,
- vid: number = 1
+ notebookContent: any
): Observable<StoreNotebookResponse> {
if (!this.enabled) {
return of({ success: false, message: "Notebook migration feature is disabled" });
@@ -237,9 +236,10 @@ export class NotebookMigrationService {
const dbAPIUrl = `${AppSettings.getApiEndpoint()}/notebook-migration/store-notebook-and-mapping`;
const headers = new HttpHeaders({ "Content-Type": "application/json" });
+ // The mapping's version id (vid) is resolved server-side from the workflow's
+ // latest version to anchor its FK, so no vid is sent from here.
const payload = {
wid,
- vid,
mapping: mappingContent,
notebook: notebookContent,
};
diff --git a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
index 1600c4c61a..3aa0c357c9 100644
--- a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
+++ b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/NotebookMigrationResource.scala
@@ -28,8 +28,10 @@ import org.apache.texera.auth.SessionUser
import org.apache.texera.dao.SqlServer
import org.jooq.JSONB
import org.jooq.exception.DataAccessException
+import org.jooq.impl.DSL
import org.apache.texera.dao.jooq.generated.tables.Notebook
import org.apache.texera.dao.jooq.generated.tables.WorkflowNotebookMapping
+import org.apache.texera.dao.jooq.generated.tables.WorkflowVersion
import java.net.{HttpURLConnection, URL}
import java.nio.charset.StandardCharsets
import scala.util.control.NonFatal
@@ -326,7 +328,6 @@ object NotebookMigrationResource extends LazyLogging {
case Left(badRequest) => return badRequest
case Right(w) => w
}
- val vid: java.lang.Integer = json.get("vid").asInt()
val mappingNode = json.get("mapping")
val notebookNode = json.get("notebook")
@@ -342,7 +343,8 @@ object NotebookMigrationResource extends LazyLogging {
// notebook.wid is UNIQUE: a workflow has at most one notebook. If one already
// exists, reject the re-store with a 409 rather than letting the INSERT trip the
- // constraint and surface as a 500.
+ // constraint and surface as a 500. Checked before the version lookup so a re-store
+ // skips that query.
val alreadyStored = dsl.fetchExists(
dsl.selectFrom(Notebook.NOTEBOOK).where(Notebook.NOTEBOOK.WID.eq(wid))
)
@@ -353,6 +355,21 @@ object NotebookMigrationResource extends LazyLogging {
.build()
}
+ // The mapping's vid FK must reference a real workflow_version row. Anchor it to the
+ // workflow's own latest version (created alongside the workflow) rather than a
+ // hardcoded id, so an unrelated workflow's version can never own or cascade it.
+ val vid: java.lang.Integer = dsl
+ .select(DSL.max(WorkflowVersion.WORKFLOW_VERSION.VID))
+ .from(WorkflowVersion.WORKFLOW_VERSION)
+ .where(WorkflowVersion.WORKFLOW_VERSION.WID.eq(wid))
+ .fetchOne(0, classOf[java.lang.Integer])
+ if (vid == null) {
+ return Response
+ .status(Response.Status.BAD_REQUEST)
+ .entity(errorJson(s"No workflow version exists for workflow $wid"))
+ .build()
+ }
+
val nid: java.lang.Integer = SqlServer.withTransaction(dsl) { ctx =>
// Insert notebook
val notebookRecord = ctx
@@ -420,8 +437,6 @@ object NotebookMigrationResource extends LazyLogging {
case Left(badRequest) => return badRequest
case Right(w) => w
}
- val vid: java.lang.Integer = json.get("vid").asInt()
-
// Only a user with write access to the workflow may fetch its notebook.
if (!WorkflowAccessResource.hasWriteAccess(wid, uid)) {
return Response
@@ -432,7 +447,10 @@ object NotebookMigrationResource extends LazyLogging {
val dsl = SqlServer.getInstance().createDSLContext()
- // Fetch the most recent notebook (highest nid) for this workflow version
+ // Fetch the notebook for this workflow, regardless of its version.
+ //
+ // Future work: to support one notebook per workflow version, drop the notebook.wid
+ // UNIQUE constraint and add a vid filter here.
val result = dsl
.select(
Notebook.NOTEBOOK.NID,
@@ -444,7 +462,6 @@ object NotebookMigrationResource extends LazyLogging {
.on(Notebook.NOTEBOOK.WID.eq(WorkflowNotebookMapping.WORKFLOW_NOTEBOOK_MAPPING.WID))
.and(Notebook.NOTEBOOK.NID.eq(WorkflowNotebookMapping.WORKFLOW_NOTEBOOK_MAPPING.NID))
.where(Notebook.NOTEBOOK.WID.eq(wid))
- .and(WorkflowNotebookMapping.WORKFLOW_NOTEBOOK_MAPPING.VID.eq(vid))
.orderBy(Notebook.NOTEBOOK.NID.desc()) // most recent nid first
.limit(1) // only take the latest
.fetchOne()
diff --git a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
index 90b24f5349..09a978efcb 100644
--- a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
+++ b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
@@ -155,15 +155,16 @@ class NotebookMigrationResourceSpec
getDSLContext.deleteFrom(USER).where(USER.EMAIL.in(writerEmail, readerEmail)).execute()
}
+ // The endpoints resolve the vid server-side (store) or ignore it (fetch), so the
+ // client sends only the wid — no vid field, matching the real frontend requests.
private def storePayload(
notebook: String = sampleNotebook,
- mapping: String = sampleMapping,
- vid: Integer = seededVid
+ mapping: String = sampleMapping
): String =
- s"""{"wid": $testWid, "vid": $vid, "notebook": $notebook, "mapping": $mapping}"""
+ s"""{"wid": $testWid, "notebook": $notebook, "mapping": $mapping}"""
- private def fetchPayload(vid: Integer = seededVid): String =
- s"""{"wid": $testWid, "vid": $vid}"""
+ private def fetchPayload(): String =
+ s"""{"wid": $testWid}"""
private def deletePayload(): String =
s"""{"wid": $testWid}"""
@@ -234,6 +235,7 @@ class NotebookMigrationResourceSpec
val mappingRow = getDSLContext.selectFrom(WORKFLOW_NOTEBOOK_MAPPING).fetchOne()
mappingRow.get(WORKFLOW_NOTEBOOK_MAPPING.WID) shouldBe testWid
+ // vid is resolved server-side to the workflow's latest version (the only seeded one here).
mappingRow.get(WORKFLOW_NOTEBOOK_MAPPING.VID) shouldBe seededVid
// The mapping row must reference the just-inserted notebook by its returned nid.
mappingRow.get(WORKFLOW_NOTEBOOK_MAPPING.NID) shouldBe notebookRow.get(NOTEBOOK.NID)
@@ -271,18 +273,26 @@ class NotebookMigrationResourceSpec
storedMappingJson should include("\"cell1\"")
}
- it should "roll back the notebook insert when the mapping insert fails its FK constraint" in {
- // workflow_notebook_mapping.vid has FK -> workflow_version(vid). Passing an
- // unknown vid trips the mapping insert; because both inserts share a single
- // SqlServer.withTransaction block, the notebook insert must roll back too.
- // Without this guarantee, orphaned notebook rows would accumulate on every
- // failed store.
- val unknownVid: Integer = -1
- val response = NotebookMigrationResource.storeNotebookAndMapping(
- storePayload(vid = unknownVid),
- writerUid
- )
- response.getStatus shouldBe Response.Status.INTERNAL_SERVER_ERROR.getStatusCode
+ it should "ignore a client-supplied vid and anchor the mapping to the workflow's latest version" in {
+ // The vid FK is resolved server-side to MAX(workflow_version.vid) for the wid, so a
+ // stale or bogus vid in the request body must never reach the mapping row. This pins
+ // the fix for the old hardcoded vid=1 behaviour.
+ val payload =
+ s"""{"wid": $testWid, "vid": 999999, "notebook": $sampleNotebook, "mapping": $sampleMapping}"""
+ val response = NotebookMigrationResource.storeNotebookAndMapping(payload, writerUid)
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+
+ val mappingRow = getDSLContext.selectFrom(WORKFLOW_NOTEBOOK_MAPPING).fetchOne()
+ mappingRow.get(WORKFLOW_NOTEBOOK_MAPPING.VID) shouldBe seededVid
+ }
+
+ it should "return 400 and store nothing when the workflow has no version to anchor the mapping" in {
+ // The mapping's vid FK needs a real workflow_version row. With none, the store must
+ // fail cleanly with a 400 before any insert, not a 500 from the FK constraint.
+ getDSLContext.deleteFrom(WORKFLOW_VERSION).where(WORKFLOW_VERSION.WID.eq(testWid)).execute()
+
+ val response = NotebookMigrationResource.storeNotebookAndMapping(storePayload(), writerUid)
+ response.getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
getDSLContext.fetchCount(NOTEBOOK) shouldBe 0
getDSLContext.fetchCount(WORKFLOW_NOTEBOOK_MAPPING) shouldBe 0
}
@@ -303,7 +313,7 @@ class NotebookMigrationResourceSpec
// -- fetchNotebookAndMapping ------------------------------------------------
- "fetchNotebookAndMapping" should "return exists=false when no notebook is stored for the (wid, vid)" in {
+ "fetchNotebookAndMapping" should "return exists=false when no notebook is stored for the workflow" in {
val response = NotebookMigrationResource.fetchNotebookAndMapping(fetchPayload(), writerUid)
response.getStatus shouldBe Response.Status.OK.getStatusCode
response.getEntity.toString should include("\"exists\": false")
@@ -322,7 +332,7 @@ class NotebookMigrationResourceSpec
entity should include("\"mapping\":")
}
- it should "return the stored notebook content for a (wid, vid) on fetch" in {
+ it should "return the stored notebook content for the workflow on fetch" in {
// notebook.wid is UNIQUE — one notebook per workflow — so the endpoint's
// orderBy(NID.desc).limit(1) resolves to that single row. This pins the
// workflow-reopen path: after a store, fetch must return that notebook's content.
@@ -342,6 +352,27 @@ class NotebookMigrationResourceSpec
entity should include("\"v1\"")
}
+ it should "return the notebook regardless of the workflow's current version" in {
+ // The mapping is stored under the version present at store time; the workflow may then
+ // advance to a newer version. Fetch keys on wid alone, so the notebook still reattaches.
+ // This mirrors the reopen-after-edit path.
+ NotebookMigrationResource.storeNotebookAndMapping(storePayload(), writerUid)
+
+ val newerVersion = new WorkflowVersion
+ newerVersion.setWid(testWid)
+ newerVersion.setContent("{}")
+ newerVersion.setCreationTime(new Timestamp(System.currentTimeMillis()))
+ workflowVersionDao.insert(newerVersion)
+ newerVersion.getVid.intValue() should be > seededVid.intValue()
+
+ val entity =
+ NotebookMigrationResource
+ .fetchNotebookAndMapping(fetchPayload(), writerUid)
+ .getEntity
+ .toString
+ entity should include("\"exists\": true")
+ }
+
// -- deleteNotebookAndMapping -----------------------------------------------
"deleteNotebookAndMapping" should "remove the notebook and cascade to its mapping, reporting deleted=1" in {
@@ -387,7 +418,7 @@ class NotebookMigrationResourceSpec
"store/fetch/delete" should "return 400 Bad Request when 'wid' is missing from the body" in {
// A missing wid must be a client error, not a 500 from the null.asInt() NPE.
- val noWid = s"""{"vid": $seededVid}"""
+ val noWid = """{"notebook": {}, "mapping": {}}"""
NotebookMigrationResource
.storeNotebookAndMapping(noWid, writerUid)
.getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
@@ -403,7 +434,7 @@ class NotebookMigrationResourceSpec
it should "return 400 Bad Request when 'wid' is not an integer" in {
// A non-integer wid must be rejected rather than silently coerced to 0 by asInt().
- val badWid = s"""{"wid": "not-an-int", "vid": $seededVid}"""
+ val badWid = """{"wid": "not-an-int"}"""
NotebookMigrationResource
.storeNotebookAndMapping(badWid, writerUid)
.getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode
@@ -472,6 +503,7 @@ class NotebookMigrationResourceSpec
val user = sessionUser(writerUid)
val badRequest = Response.Status.BAD_REQUEST.getStatusCode
resource.setNotebook("not json", user).getStatus shouldBe badRequest
+ resource.storeNotebookAndMapping("not json", user).getStatus shouldBe badRequest
resource.fetchNotebookAndMapping("not json", user).getStatus shouldBe badRequest
}