From 846454306ec69a384f8f1d91dbcbd75982166934 Mon Sep 17 00:00:00 2001 From: Junxiang Chen Date: Wed, 18 Dec 2024 14:32:17 +0000 Subject: [PATCH 1/7] test: add test case --- .vscode/settings.json | 21 +++ package-lock.json | 12 +- .../test/validation/error-validation.test.ts | 11 ++ .../test-schemas/duplicated-node-labels.json | 123 ++++++++++++++++++ 4 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 packages/graph-schema-utils/test/validation/test-schemas/duplicated-node-labels.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..af9d4c6 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,21 @@ +{ + "editor.formatOnSave": true, + "[javascript]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "[typescript]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "[javascriptreact]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "[typescriptreact]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "[json]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "[jsonc]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + } +} \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index f708569..5f7208b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4449,13 +4449,13 @@ }, "packages/graph-schema-utils": { "name": "@neo4j/graph-schema-utils", - "version": "1.0.0-next.15", + "version": "1.0.0-next.16", "license": "Apache-2.0", "dependencies": { "ajv": "^8.11.0" }, "devDependencies": { - "@neo4j/graph-json-schema": "1.0.0-next.4" + "@neo4j/graph-json-schema": "1.0.0-next.5" } }, "packages/introspection": { @@ -4463,13 +4463,13 @@ "version": "1.0.0-next.2", "license": "Apache-2.0", "devDependencies": { - "@neo4j/graph-json-schema": "1.0.0-next.4", + "@neo4j/graph-json-schema": "1.0.0-next.5", "neo4j-driver": "^5.12.0" } }, "packages/json-schema": { "name": "@neo4j/graph-json-schema", - "version": "1.0.0-next.4", + "version": "1.0.0-next.5", "license": "Apache-2.0" } }, @@ -5012,7 +5012,7 @@ "@neo4j/graph-introspection": { "version": "file:packages/introspection", "requires": { - "@neo4j/graph-json-schema": "1.0.0-next.4", + "@neo4j/graph-json-schema": "1.0.0-next.5", "neo4j-driver": "^5.12.0" } }, @@ -5022,7 +5022,7 @@ "@neo4j/graph-schema-utils": { "version": "file:packages/graph-schema-utils", "requires": { - "@neo4j/graph-json-schema": "1.0.0-next.4", + "@neo4j/graph-json-schema": "1.0.0-next.5", "ajv": "^8.11.0" } }, diff --git a/packages/graph-schema-utils/test/validation/error-validation.test.ts b/packages/graph-schema-utils/test/validation/error-validation.test.ts index 49de003..e1d6c72 100644 --- a/packages/graph-schema-utils/test/validation/error-validation.test.ts +++ b/packages/graph-schema-utils/test/validation/error-validation.test.ts @@ -12,6 +12,17 @@ const JSON_SCHEMA = JSON.stringify( // Validate type errors == schemas we expect NOT to pass describe("Validate type errors", () => { + describe("Identifies duplicated node labels", () => { + test("Duplicated node labels", () => { + const duplicatedNodeLabels = readFile( + path.resolve(__dirname, "./test-schemas/duplicated-node-labels.json") + ); + assert.throws( + () => validateSchema(JSON_SCHEMA, duplicatedNodeLabels), + SchemaValidationError + ); + }); + }); describe("Identifies unsupported types", () => { test("Node object types", () => { const unsupportedNodeSpecs = readFile( diff --git a/packages/graph-schema-utils/test/validation/test-schemas/duplicated-node-labels.json b/packages/graph-schema-utils/test/validation/test-schemas/duplicated-node-labels.json new file mode 100644 index 0000000..f5c1cbc --- /dev/null +++ b/packages/graph-schema-utils/test/validation/test-schemas/duplicated-node-labels.json @@ -0,0 +1,123 @@ +{ + "graphSchemaRepresentation": { + "version": "1.0.0", + "graphSchema": { + "nodeLabels": [ + { + "$id": "nl:0", + "token": "DOCS_CHUNKS_TABLE", + "properties": [ + { + "$id": "p:0_0", + "token": "RELATIVE_PATH", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_1", + "token": "SIZE", + "type": { + "type": "integer" + }, + "nullable": false + }, + { + "$id": "p:0_2", + "token": "FILE_URL", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_3", + "token": "SCOPED_FILE_URL", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_4", + "token": "CHUNK", + "type": { + "type": "string" + }, + "nullable": false + } + ] + }, + { + "$id": "nl:1", + "token": "DOCS_METADATA", + "properties": [ + { + "$id": "p:1_0", + "token": "RELATIVE_PATH", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:1_1", + "token": "MODEL_NAME", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:1_2", + "token": "SKILLS", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:1_3", + "token": "DOMAINS", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:1_4", + "token": "SUMMARY", + "type": { + "type": "string" + }, + "nullable": false + } + ] + } + ], + "relationshipTypes": [], + "nodeObjectTypes": [ + { + "$id": "n:0", + "labels": [ + { + "$ref": "#nl:0" + } + ] + }, + { + "$id": "n:1", + "labels": [ + { + "$ref": "#nl:1" + } + ] + } + ], + "relationshipObjectTypes": [], + "constraints": [], + "indexes": [] + } + } +} From 08d4ab12b4158e0f415b88f3070ad8598295f83f Mon Sep 17 00:00:00 2001 From: Junxiang Chen Date: Wed, 18 Dec 2024 14:45:57 +0000 Subject: [PATCH 2/7] test: create a new test file for better structure --- .../src/formatters/json/extensions.test.ts | 16 ++++++++++++++++ .../test-schemas/duplicated-nodeLabel-ids.json} | 0 .../test/validation/error-validation.test.ts | 11 ----------- 3 files changed, 16 insertions(+), 11 deletions(-) create mode 100644 packages/graph-schema-utils/src/formatters/json/extensions.test.ts rename packages/graph-schema-utils/{test/validation/test-schemas/duplicated-node-labels.json => src/formatters/json/test-schemas/duplicated-nodeLabel-ids.json} (100%) diff --git a/packages/graph-schema-utils/src/formatters/json/extensions.test.ts b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts new file mode 100644 index 0000000..9e677f9 --- /dev/null +++ b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts @@ -0,0 +1,16 @@ +import { describe, test } from "vitest"; +import { readFile } from "../../../test/fs.utils.js"; +import path from "path"; +import { fromJson } from "./index.js"; +import { strict as assert } from "node:assert"; + +describe("JSON formatter", () => { + describe("fromJsonStruct", () => { + const schemaWithDuplicatedNodeLabels = readFile( + path.resolve(__dirname, "./test-schemas/duplicated-nodeLabel-ids.json") + ); + test("Identifies duplicated node labels adn throws an error", () => { + assert.throws(() => fromJson(schemaWithDuplicatedNodeLabels)); + }); + }); +}); diff --git a/packages/graph-schema-utils/test/validation/test-schemas/duplicated-node-labels.json b/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeLabel-ids.json similarity index 100% rename from packages/graph-schema-utils/test/validation/test-schemas/duplicated-node-labels.json rename to packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeLabel-ids.json diff --git a/packages/graph-schema-utils/test/validation/error-validation.test.ts b/packages/graph-schema-utils/test/validation/error-validation.test.ts index e1d6c72..49de003 100644 --- a/packages/graph-schema-utils/test/validation/error-validation.test.ts +++ b/packages/graph-schema-utils/test/validation/error-validation.test.ts @@ -12,17 +12,6 @@ const JSON_SCHEMA = JSON.stringify( // Validate type errors == schemas we expect NOT to pass describe("Validate type errors", () => { - describe("Identifies duplicated node labels", () => { - test("Duplicated node labels", () => { - const duplicatedNodeLabels = readFile( - path.resolve(__dirname, "./test-schemas/duplicated-node-labels.json") - ); - assert.throws( - () => validateSchema(JSON_SCHEMA, duplicatedNodeLabels), - SchemaValidationError - ); - }); - }); describe("Identifies unsupported types", () => { test("Node object types", () => { const unsupportedNodeSpecs = readFile( From 29cdece44ecd92ec99a9e1d1c4d3e8420ebe255e Mon Sep 17 00:00:00 2001 From: Junxiang Chen Date: Wed, 18 Dec 2024 14:58:17 +0000 Subject: [PATCH 3/7] test: add test case for duplicated node label ids --- .../src/formatters/json/extensions.test.ts | 6 ++++-- .../src/formatters/json/extensions.ts | 20 +++++++++++++++++++ .../duplicated-nodeLabel-ids.json | 16 +++++++-------- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/packages/graph-schema-utils/src/formatters/json/extensions.test.ts b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts index 9e677f9..7d1a5c4 100644 --- a/packages/graph-schema-utils/src/formatters/json/extensions.test.ts +++ b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts @@ -5,12 +5,14 @@ import { fromJson } from "./index.js"; import { strict as assert } from "node:assert"; describe("JSON formatter", () => { - describe("fromJsonStruct", () => { + describe("fromJson", () => { const schemaWithDuplicatedNodeLabels = readFile( path.resolve(__dirname, "./test-schemas/duplicated-nodeLabel-ids.json") ); test("Identifies duplicated node labels adn throws an error", () => { - assert.throws(() => fromJson(schemaWithDuplicatedNodeLabels)); + assert.throws(() => fromJson(schemaWithDuplicatedNodeLabels), { + message: "Duplicate node label IDs found in schema", + }); }); }); }); diff --git a/packages/graph-schema-utils/src/formatters/json/extensions.ts b/packages/graph-schema-utils/src/formatters/json/extensions.ts index abe6ab2..797dcd0 100644 --- a/packages/graph-schema-utils/src/formatters/json/extensions.ts +++ b/packages/graph-schema-utils/src/formatters/json/extensions.ts @@ -23,6 +23,7 @@ import { } from "../../model/index.js"; import { ConstraintJsonStruct, + GraphSchemaJsonStruct, IndexJsonStruct, isLookupIndexJsonStruct, isNodeLabelConstraintJsonStruct, @@ -107,8 +108,27 @@ export function fromJson(schema: string): GraphSchema { return fromJsonStruct(schemaJson); } +export function hasDuplicateNodeLabelIds( + schema: GraphSchemaJsonStruct +): boolean { + const ids = new Set(); + + for (const nodeLabel of schema.nodeLabels) { + if (ids.has(nodeLabel.$id)) { + return true; + } + ids.add(nodeLabel.$id); + } + + return false; +} + export function fromJsonStruct(schemaJson: RootSchemaJsonStruct): GraphSchema { const { graphSchema } = schemaJson.graphSchemaRepresentation; + console.log(graphSchema, hasDuplicateNodeLabelIds(graphSchema)); + if (hasDuplicateNodeLabelIds(graphSchema)) { + throw new Error("Duplicate node label IDs found in schema"); + } const labels = graphSchema.nodeLabels.map(nodeLabel.create); const relationshipTypes = graphSchema.relationshipTypes.map( relationshipType.create diff --git a/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeLabel-ids.json b/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeLabel-ids.json index f5c1cbc..a5f3121 100644 --- a/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeLabel-ids.json +++ b/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeLabel-ids.json @@ -50,11 +50,11 @@ ] }, { - "$id": "nl:1", + "$id": "nl:0", "token": "DOCS_METADATA", "properties": [ { - "$id": "p:1_0", + "$id": "p:0_0", "token": "RELATIVE_PATH", "type": { "type": "string" @@ -62,7 +62,7 @@ "nullable": false }, { - "$id": "p:1_1", + "$id": "p:0_1", "token": "MODEL_NAME", "type": { "type": "string" @@ -70,7 +70,7 @@ "nullable": false }, { - "$id": "p:1_2", + "$id": "p:0_2", "token": "SKILLS", "type": { "type": "string" @@ -78,7 +78,7 @@ "nullable": false }, { - "$id": "p:1_3", + "$id": "p:0_3", "token": "DOMAINS", "type": { "type": "string" @@ -86,7 +86,7 @@ "nullable": false }, { - "$id": "p:1_4", + "$id": "p:0_4", "token": "SUMMARY", "type": { "type": "string" @@ -107,10 +107,10 @@ ] }, { - "$id": "n:1", + "$id": "n:0", "labels": [ { - "$ref": "#nl:1" + "$ref": "#nl:0" } ] } From 86c4c4879d0f521b5b4685f57cddf62b0acb8f70 Mon Sep 17 00:00:00 2001 From: Junxiang Chen Date: Wed, 18 Dec 2024 15:02:29 +0000 Subject: [PATCH 4/7] test: add test for duplicated node object type ids --- .../src/formatters/json/extensions.test.ts | 19 +++ .../src/formatters/json/extensions.ts | 18 +++ .../duplicated-nodeObjectType-ids.json | 123 ++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeObjectType-ids.json diff --git a/packages/graph-schema-utils/src/formatters/json/extensions.test.ts b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts index 7d1a5c4..4ab87c3 100644 --- a/packages/graph-schema-utils/src/formatters/json/extensions.test.ts +++ b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts @@ -14,5 +14,24 @@ describe("JSON formatter", () => { message: "Duplicate node label IDs found in schema", }); }); + + test("Identifies duplicated node object types and throws an error", () => { + const schema = readFile( + path.resolve( + __dirname, + "./test-schemas/duplicated-nodeObjectType-ids.json" + ) + ); + assert.throws(() => fromJson(schema), { + message: "Duplicate node object type IDs found in schema", + }); + }); + + test("Does not throw an error if there are no duplicated ids in node labels or node object types", () => { + const schema = readFile( + path.resolve(__dirname, "./test-schemas/full.json") + ); + assert.doesNotThrow(() => fromJson(schema)); + }); }); }); diff --git a/packages/graph-schema-utils/src/formatters/json/extensions.ts b/packages/graph-schema-utils/src/formatters/json/extensions.ts index 797dcd0..b4be734 100644 --- a/packages/graph-schema-utils/src/formatters/json/extensions.ts +++ b/packages/graph-schema-utils/src/formatters/json/extensions.ts @@ -123,6 +123,21 @@ export function hasDuplicateNodeLabelIds( return false; } +export function hasDuplicateNodeObjectTypeIds( + schema: GraphSchemaJsonStruct +): boolean { + const ids = new Set(); + + for (const nodeObjectType of schema.nodeObjectTypes) { + if (ids.has(nodeObjectType.$id)) { + return true; + } + ids.add(nodeObjectType.$id); + } + + return false; +} + export function fromJsonStruct(schemaJson: RootSchemaJsonStruct): GraphSchema { const { graphSchema } = schemaJson.graphSchemaRepresentation; console.log(graphSchema, hasDuplicateNodeLabelIds(graphSchema)); @@ -133,6 +148,9 @@ export function fromJsonStruct(schemaJson: RootSchemaJsonStruct): GraphSchema { const relationshipTypes = graphSchema.relationshipTypes.map( relationshipType.create ); + if (hasDuplicateNodeObjectTypeIds(graphSchema)) { + throw new Error("Duplicate node object type IDs found in schema"); + } const nodeObjectTypes = graphSchema.nodeObjectTypes.map( (nodeObjectTypeJson) => nodeObjectType.create(nodeObjectTypeJson, (ref) => { diff --git a/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeObjectType-ids.json b/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeObjectType-ids.json new file mode 100644 index 0000000..cc3f323 --- /dev/null +++ b/packages/graph-schema-utils/src/formatters/json/test-schemas/duplicated-nodeObjectType-ids.json @@ -0,0 +1,123 @@ +{ + "graphSchemaRepresentation": { + "version": "1.0.0", + "graphSchema": { + "nodeLabels": [ + { + "$id": "nl:0", + "token": "DOCS_CHUNKS_TABLE", + "properties": [ + { + "$id": "p:0_0", + "token": "RELATIVE_PATH", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_1", + "token": "SIZE", + "type": { + "type": "integer" + }, + "nullable": false + }, + { + "$id": "p:0_2", + "token": "FILE_URL", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_3", + "token": "SCOPED_FILE_URL", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_4", + "token": "CHUNK", + "type": { + "type": "string" + }, + "nullable": false + } + ] + }, + { + "$id": "nl:1", + "token": "DOCS_METADATA", + "properties": [ + { + "$id": "p:0_0", + "token": "RELATIVE_PATH", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_1", + "token": "MODEL_NAME", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_2", + "token": "SKILLS", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_3", + "token": "DOMAINS", + "type": { + "type": "string" + }, + "nullable": false + }, + { + "$id": "p:0_4", + "token": "SUMMARY", + "type": { + "type": "string" + }, + "nullable": false + } + ] + } + ], + "relationshipTypes": [], + "nodeObjectTypes": [ + { + "$id": "n:0", + "labels": [ + { + "$ref": "#nl:0" + } + ] + }, + { + "$id": "n:0", + "labels": [ + { + "$ref": "#nl:0" + } + ] + } + ], + "relationshipObjectTypes": [], + "constraints": [], + "indexes": [] + } + } +} From 0317cf7b88e7a76c6c7ac33c00b7ee05dd3e3633 Mon Sep 17 00:00:00 2001 From: Junxiang Chen Date: Wed, 18 Dec 2024 15:05:01 +0000 Subject: [PATCH 5/7] test: unit test on unit functions --- .../src/formatters/json/extensions.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/graph-schema-utils/src/formatters/json/extensions.test.ts b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts index 4ab87c3..cab91fe 100644 --- a/packages/graph-schema-utils/src/formatters/json/extensions.test.ts +++ b/packages/graph-schema-utils/src/formatters/json/extensions.test.ts @@ -3,8 +3,46 @@ import { readFile } from "../../../test/fs.utils.js"; import path from "path"; import { fromJson } from "./index.js"; import { strict as assert } from "node:assert"; +import { + hasDuplicateNodeLabelIds, + hasDuplicateNodeObjectTypeIds, +} from "./extensions.js"; +import { RootSchemaJsonStruct } from "./types.js"; describe("JSON formatter", () => { + describe("hasDuplicateNodeLabelIds", () => { + test("Identifies duplicated node labels", () => { + const schema = readFile( + path.resolve(__dirname, "./test-schemas/duplicated-nodeLabel-ids.json") + ); + const schemaJson = JSON.parse(schema) as RootSchemaJsonStruct; + + assert.strictEqual( + hasDuplicateNodeLabelIds( + schemaJson.graphSchemaRepresentation.graphSchema + ), + true + ); + }); + }); + describe("hasDuplicateNodeObjectTypeIds", () => { + test("Identifies duplicated node object types", () => { + const schema = readFile( + path.resolve( + __dirname, + "./test-schemas/duplicated-nodeObjectType-ids.json" + ) + ); + const schemaJson = JSON.parse(schema) as RootSchemaJsonStruct; + + assert.strictEqual( + hasDuplicateNodeObjectTypeIds( + schemaJson.graphSchemaRepresentation.graphSchema + ), + true + ); + }); + }); describe("fromJson", () => { const schemaWithDuplicatedNodeLabels = readFile( path.resolve(__dirname, "./test-schemas/duplicated-nodeLabel-ids.json") From 40d96a8f24cf50f4525c9a388c2f3a72b8bcb168 Mon Sep 17 00:00:00 2001 From: Junxiang Chen Date: Thu, 19 Dec 2024 10:12:29 +0000 Subject: [PATCH 6/7] chore: remove console.log --- packages/graph-schema-utils/src/formatters/json/extensions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/graph-schema-utils/src/formatters/json/extensions.ts b/packages/graph-schema-utils/src/formatters/json/extensions.ts index b4be734..00297fd 100644 --- a/packages/graph-schema-utils/src/formatters/json/extensions.ts +++ b/packages/graph-schema-utils/src/formatters/json/extensions.ts @@ -140,7 +140,6 @@ export function hasDuplicateNodeObjectTypeIds( export function fromJsonStruct(schemaJson: RootSchemaJsonStruct): GraphSchema { const { graphSchema } = schemaJson.graphSchemaRepresentation; - console.log(graphSchema, hasDuplicateNodeLabelIds(graphSchema)); if (hasDuplicateNodeLabelIds(graphSchema)) { throw new Error("Duplicate node label IDs found in schema"); } From cc4563058b7f0ad3f939f5f42bd5b8250421f8b3 Mon Sep 17 00:00:00 2001 From: Junxiang Chen Date: Thu, 19 Dec 2024 11:30:14 +0000 Subject: [PATCH 7/7] chore: add changeset --- .changeset/fifty-suns-march.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fifty-suns-march.md diff --git a/.changeset/fifty-suns-march.md b/.changeset/fifty-suns-march.md new file mode 100644 index 0000000..b51aa25 --- /dev/null +++ b/.changeset/fifty-suns-march.md @@ -0,0 +1,5 @@ +--- +"@neo4j/graph-schema-utils": patch +--- + +fix: detect malformed graph schema with duplicated node label ids and object type ids