Skip to content

Commit

Permalink
Export Item and Article parsing functions from the 'perseus' package (#…
Browse files Browse the repository at this point in the history
…2085)

This is the public API for Perseus JSON parsing that we'll call in Webapp.
See [ADR 773][1] for context on why this is needed.

[1]: https://khanacademy.atlassian.net/wiki/spaces/ENG/pages/3318349891/ADR+773+Validate+widget+data+on+input+in+Perseus

Issue: https://khanacademy.atlassian.net/browse/LEMS-2774

## Test plan:

`yarn test`

Author: benchristel

Reviewers: jeremywiebe, handeyeco, SonicScrewdriver, mark-fitzgerald

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x)

Pull Request URL: #2085
  • Loading branch information
benchristel authored Jan 11, 2025
1 parent 6cf6477 commit 72fb7ec
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/strange-buses-run.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Deprecates `parsePerseusItem()` in favor of typesafe `parseAndMigratePerseusItem()` and `parseAndMigratePerseusArticle()` functions.
8 changes: 7 additions & 1 deletion packages/perseus/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,12 @@ export {
getAnswerFromUserInput,
getImagesWithoutAltData,
} from "./util/extract-perseus-data";
export {parsePerseusItem} from "./util/parse-perseus-json";
export {
parsePerseusItem,
parseAndMigratePerseusItem,
parseAndMigratePerseusArticle,
} from "./util/parse-perseus-json";
export {isSuccess, isFailure} from "./util/parse-perseus-json/result";
export {
generateTestPerseusItem,
genericPerseusItemData,
Expand Down Expand Up @@ -191,6 +196,7 @@ export type {
SharedRendererProps,
} from "./types";
export type {ParsedValue} from "./util";
export type {Result, Success, Failure} from "./util/parse-perseus-json/result";
export type {UserInputMap} from "./validation.types";
export type {Coord} from "./interactive2/types";
export type {Coords} from "./widgets/grapher/grapher-types";
Expand Down
74 changes: 66 additions & 8 deletions packages/perseus/src/util/parse-perseus-json/index.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
import {isRealJSONParse} from "../is-real-json-parse";

import {parse} from "./parse";
import {parsePerseusItem as typecheckPerseusItem} from "./perseus-parsers/perseus-item";
import {parsePerseusArticle as migrateAndTypecheckPerseusArticle} from "./perseus-parsers/perseus-article";
import {parsePerseusItem as migrateAndTypecheckPerseusItem} from "./perseus-parsers/perseus-item";
import {failure, isFailure} from "./result";

import type {Result} from "./result";
import type {PerseusItem} from "@khanacademy/perseus-core";
import type {PerseusItem, PerseusArticle} from "@khanacademy/perseus-core";

/**
* Helper to parse PerseusItem JSON
* Why not just use JSON.parse? We want:
* - To make sure types are correct
* - To give us a central place to validate/transform output if needed
* @deprecated - use parseAndMigratePerseusItem instead
* @param {string} json - the stringified PerseusItem JSON
* @returns {PerseusItem} the parsed PerseusItem object
*/
Expand All @@ -23,13 +26,68 @@ export function parsePerseusItem(json: string): PerseusItem {
throw new Error("Something went wrong.");
}

export type ParseFailureDetail = {
/**
* A human-readable error message describing where in the object tree
* parsing failed.
*/
message: string;
/**
* The raw result of parsing the input JSON, with no migrations applied.
* Use at your own risk.
*/
invalidObject: unknown;
};

/**
* Parses a PerseusItem from a JSON string, migrates old formats to the latest
* schema, and runtime-typechecks the result. Use this to parse assessmentItem
* data.
*
* @returns a {@link Result} of the parsed PerseusItem. If the result is a
* failure, it will contain an error message describing where in the tree
* parsing failed.
* @throws SyntaxError if the argument is not well-formed JSON.
*/
export function parseAndMigratePerseusItem(
json: string,
): Result<PerseusItem, ParseFailureDetail> {
throwErrorIfCheatingDetected();
const object: unknown = JSON.parse(json);
const result = parse(object, migrateAndTypecheckPerseusItem);
if (isFailure(result)) {
return failure({message: result.detail, invalidObject: object});
}
return result;
}

/**
* Typesafe version of parsePerseusItem, which runtime-typechecks the JSON.
* TODO(benchristel): Replace parsePerseusItem with this function.
* Parses a PerseusArticle from a JSON string, migrates old formats to the
* latest schema, and runtime-typechecks the result.
*
* @returns a {@link Result} of the parsed PerseusArticle. If the result is a
* failure, it will contain an error message describing where in the tree
* parsing failed.
* @throws SyntaxError if the argument is not well-formed JSON.
*/
export function parseAndTypecheckPerseusItem(
export function parseAndMigratePerseusArticle(
json: string,
): Result<PerseusItem, string> {
const object: unknown = parsePerseusItem(json);
return parse(object, typecheckPerseusItem);
): Result<PerseusArticle, ParseFailureDetail> {
throwErrorIfCheatingDetected();
const object: unknown = JSON.parse(json);
const result = parse(object, migrateAndTypecheckPerseusArticle);
if (isFailure(result)) {
return failure({message: result.detail, invalidObject: object});
}
return result;
}

/**
* Tries to block a cheating vector that relies on monkey-patching JSON.parse.
*/
// TODO(LEMS-2331): delete this function once server-side scoring is done.
function throwErrorIfCheatingDetected() {
if (!isRealJSONParse(JSON.parse)) {
throw new Error("Something went wrong.");
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import {assertFailure, assertSuccess} from "./result";
import {jest} from "@jest/globals";

import {parseAndTypecheckPerseusItem} from ".";
import {assertFailure, assertSuccess, success} from "./result";

describe("parseAndTypecheckPerseusItem", () => {
import {parseAndMigratePerseusItem, parseAndMigratePerseusArticle} from ".";

describe("parseAndMigratePerseusItem", () => {
it("should parse JSON", () => {
const result = parseAndTypecheckPerseusItem(
const result = parseAndMigratePerseusItem(
`{
"itemDataVersion": { "major": 0, "minor": 0 },
"answerArea": {},
Expand All @@ -24,13 +26,91 @@ describe("parseAndTypecheckPerseusItem", () => {
});

it("returns an error given an invalid PerseusItem", () => {
const result = parseAndTypecheckPerseusItem(
`{"question": "bad value"}`,
);
const result = parseAndMigratePerseusItem(`{"question": "bad value"}`);

assertFailure(result);
expect(result.detail).toContain(
expect(result.detail.message).toContain(
`At (root).question -- expected object, but got "bad value"`,
);
});

it("returns the invalid object along with the error", () => {
const result = parseAndMigratePerseusItem(`{"question": "bad value"}`);

assertFailure(result);
expect(result.detail.invalidObject).toEqual({question: "bad value"});
});

it("throws an error given malformed JSON", () => {
expect(() => parseAndMigratePerseusItem("")).toThrowError(
new SyntaxError("Unexpected end of JSON input"),
);
});

it("throws an error if JSON.parse is monkey-patched", () => {
// This is an attempt to make cheating more difficult.
const validItem = `{"question": ""}`;
jest.spyOn(JSON, "parse").mockReturnValue({question: ""});
expect(() => parseAndMigratePerseusItem(validItem)).toThrowError();
});
});

describe("parseAndMigratePerseusArticle", () => {
it("parses a single renderer", () => {
const result = parseAndMigratePerseusArticle(
`{"content": "", "widgets": {}}`,
);

expect(result).toEqual(
success({
content: "",
widgets: {},
images: {},
metadata: undefined,
}),
);
});

it("parses an array of renderers", () => {
const result = parseAndMigratePerseusArticle(
`[{"content": "one"}, {"content": "two"}]`,
);
expect(result).toEqual(
success([
{content: "one", widgets: {}, images: {}, metadata: undefined},
{content: "two", widgets: {}, images: {}, metadata: undefined},
]),
);
});

it("fails given invalid data", () => {
const result = parseAndMigratePerseusArticle("[9]");

assertFailure(result);
expect(result.detail.message).toEqual(
"At (root)[0] -- expected object, but got 9",
);
});

it("returns the invalid object along with the error", () => {
const result = parseAndMigratePerseusArticle("[9]");

assertFailure(result);
expect(result.detail.invalidObject).toEqual([9]);
});

it("throws an error given malformed JSON", () => {
expect(() => parseAndMigratePerseusArticle("")).toThrowError(
new SyntaxError("Unexpected end of JSON input"),
);
});

it("throws an error if JSON.parse is monkey-patched", () => {
// This is an attempt to make cheating more difficult.
const validArticle = `{"content": "", "widgets": {}}`;
jest.spyOn(JSON, "parse").mockReturnValue({content: "", widgets: {}});
expect(() =>
parseAndMigratePerseusArticle(validArticle),
).toThrowError();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {array, union} from "../general-purpose-parsers";

import {parsePerseusRenderer} from "./perseus-renderer";

import type {Parser} from "../parser-types";
import type {PerseusArticle} from "@khanacademy/perseus-core";

export const parsePerseusArticle: Parser<PerseusArticle> = union(
parsePerseusRenderer,
).or(array(parsePerseusRenderer)).parser;
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from "fs";
import {join} from "path";

import {parseAndTypecheckPerseusItem} from "../index";
import {parseAndMigratePerseusItem} from "../index";

const dataFiles = fs.readdirSync(join(__dirname, "data"));

Expand All @@ -11,7 +11,7 @@ describe("parseAndTypecheckPerseusItem", () => {
join(__dirname, "data", filename),
"utf-8",
);
const result = parseAndTypecheckPerseusItem(json);
const result = parseAndMigratePerseusItem(json);

// This strange-looking assertion style results in the failure message
// being printed if parsing fails, so the test is easier to debug.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from "fs";
import {join} from "path";

import {parseAndTypecheckPerseusItem} from "../index";
import {parseAndMigratePerseusItem} from "../index";
import {assertSuccess} from "../result";

const dataFiles = fs.readdirSync(join(__dirname, "data"));
Expand All @@ -15,7 +15,7 @@ describe("parseAndTypecheckPerseusItem", () => {
join(__dirname, "data", filename),
"utf-8",
);
const result = parseAndTypecheckPerseusItem(json);
const result = parseAndMigratePerseusItem(json);
assertSuccess(result);
expect(result.value).toMatchSnapshot();
});
Expand Down

0 comments on commit 72fb7ec

Please sign in to comment.