From 0aedf43874a6f950614134967bf4b173e4513ba0 Mon Sep 17 00:00:00 2001 From: Matt Kornatz Date: Sat, 10 Oct 2020 15:23:57 -0400 Subject: [PATCH] fix: Allows valid non-object JSON to be retrieved in simple-json columns (#6574) Fixes #5501 --- src/util/DateUtils.ts | 9 +- test/github-issues/4440/issue-4440.ts | 43 ------- .../{4440 => 5501}/entity/Post.ts | 1 - test/github-issues/5501/issue-5501.ts | 112 ++++++++++++++++++ 4 files changed, 114 insertions(+), 51 deletions(-) delete mode 100644 test/github-issues/4440/issue-4440.ts rename test/github-issues/{4440 => 5501}/entity/Post.ts (99%) create mode 100644 test/github-issues/5501/issue-5501.ts diff --git a/src/util/DateUtils.ts b/src/util/DateUtils.ts index dcc649d766..8f3b99a02d 100644 --- a/src/util/DateUtils.ts +++ b/src/util/DateUtils.ts @@ -1,4 +1,4 @@ -import {ColumnMetadata} from "../metadata/ColumnMetadata"; +import { ColumnMetadata } from "../metadata/ColumnMetadata"; /** * Provides utilities to transform hydrated and persisted data. @@ -175,12 +175,7 @@ export class DateUtils { } static stringToSimpleJson(value: any) { - try { - const simpleJSON = JSON.parse(value); - return (typeof simpleJSON === "object") ? simpleJSON : {}; - } catch (err) { - return {}; - } + return typeof value === "string" ? JSON.parse(value) : value; } static simpleEnumToString(value: any) { diff --git a/test/github-issues/4440/issue-4440.ts b/test/github-issues/4440/issue-4440.ts deleted file mode 100644 index e42ca2eb30..0000000000 --- a/test/github-issues/4440/issue-4440.ts +++ /dev/null @@ -1,43 +0,0 @@ -import "reflect-metadata"; -import { Connection } from "../../../src/connection/Connection"; -import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils"; -import { Post } from "./entity/Post"; - -describe("github issues > #4440 simple-json column type throws error for string with no value", () => { - - let connections: Connection[]; - before(async () => { - connections = await createTestingConnections({ - entities: [Post], - schemaCreate: true, - dropSchema: true - }); - }); - beforeEach(() => reloadTestingDatabases(connections)); - after(() => closeTestingConnections(connections)); - - it("should correctly add retrieve simple-json field with no value", () => - Promise.all(connections.map(async (connection) => { - const repo = connection.getRepository(Post); - const post = new Post(); - post.id = 1; - post.jsonField = ""; - await repo.save(post); - const postFound = await repo.findOne(1); - postFound!.id.should.eql(1); - postFound!.jsonField.should.eql({}); - }))); - - it("should correctly add retrieve simple-json field with some value", () => - Promise.all(connections.map(async (connection) => { - const repo = connection.getRepository(Post); - const post = new Post(); - post.id = 1; - post.jsonField = {"key": "value"}; - await repo.save(post); - const postFound = await repo.findOne(1); - postFound!.id.should.eql(1); - postFound!.jsonField.should.eql({"key": "value"}); - }))); - -}); diff --git a/test/github-issues/4440/entity/Post.ts b/test/github-issues/5501/entity/Post.ts similarity index 99% rename from test/github-issues/4440/entity/Post.ts rename to test/github-issues/5501/entity/Post.ts index 5220e523cd..72dbebb2ff 100644 --- a/test/github-issues/4440/entity/Post.ts +++ b/test/github-issues/5501/entity/Post.ts @@ -12,5 +12,4 @@ export class Post { nullable: true }) jsonField: any; - } diff --git a/test/github-issues/5501/issue-5501.ts b/test/github-issues/5501/issue-5501.ts new file mode 100644 index 0000000000..b5b0c3cb11 --- /dev/null +++ b/test/github-issues/5501/issue-5501.ts @@ -0,0 +1,112 @@ +import "reflect-metadata"; +import { Connection } from "../../../src/connection/Connection"; +import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils"; +import { Post } from "./entity/Post"; +import { expect } from "chai"; + +describe("github issues > #5501 Incorrect data loading from JSON string for column type 'simple-json'", () => { + + let connections: Connection[]; + before(async () => { + connections = await createTestingConnections({ + entities: [Post], + schemaCreate: true, + dropSchema: true + }); + }); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should correctly store simple-json field", () => Promise.all(connections.map(async (connection) => { + let id = 0; + const runTestCase = async (input: any, expected: any, message: string) => { + id++; + + await connection.getRepository(Post).save({ id , jsonField: input }); + + const actual = ( + await connection.createQueryBuilder() + .from("Post", "post") + .select("post.jsonField", "json") + .where("post.id = :id", {id}) + .getRawOne() + )!.json; + + expect(actual).to.be.equal(expected, message); + } + + await runTestCase("hello world", "\"hello world\"", "normal string"); + await runTestCase("", "\"\"", "empty string"); + await runTestCase("null", "\"null\"", "string containing the word null"); + await runTestCase( { "key": "value" }, "{\"key\":\"value\"}", "object containing a key and string value"); + await runTestCase([ "hello" ], "[\"hello\"]", "array containing a string"); + await runTestCase(null, null, "a null object value"); + await runTestCase(1, "1", "the real number 1"); + await runTestCase(0.3, "0.3", "the number 0.3"); + await runTestCase(true, "true", "the boolean value true"); + await runTestCase( + [ { hello: "earth", planet: true }, { hello: "moon", planet: false } ], + "[{\"hello\":\"earth\",\"planet\":true},{\"hello\":\"moon\",\"planet\":false}]", + "a complex object example" + ); + }))); + + it("should correctly retrieve simple-json field", () => Promise.all(connections.map(async (connection) => { + let id = 0; + const runTestCase = async (input: string | null, expected: any, message: string) => { + id++; + await connection.createQueryBuilder() + .insert() + .into(Post) + .values({id, jsonField: () => ':field'} as any) // A bit of a hack to get the raw value inserting + .setParameter('field', input) + .execute(); + + const actual = ( + await connection.getRepository(Post).findOne({ where: { id } }) + )!.jsonField; + + expect(actual).to.be.eql(expected, message); + } + + await runTestCase("\"hello world\"", "hello world", "normal string"); + await runTestCase("\"\"", "", "empty string"); + await runTestCase("\"null\"", "null", "string containing the word null"); + await runTestCase("{\"key\":\"value\"}", { "key": "value" }, "object containing a key and string value"); + await runTestCase("[\"hello\"]", [ "hello" ], "array containing a string"); + await runTestCase(null, null, "a null object value");; + await runTestCase("1", 1, "the real number 1"); + await runTestCase("0.3", 0.3, "the number 0.3"); + await runTestCase("true", true, "the boolean value true"); + await runTestCase( + "[{\"hello\":\"earth\",\"planet\":true},{\"hello\":\"moon\",\"planet\":false}]", + [{"hello":"earth","planet":true},{"hello":"moon","planet":false}], + "a complex object example" + ); + }))); + + it("should throw an error when the data in the database is invalid", () => Promise.all(connections.map(async (connection) => { + const insert = (id: number, value: string | null) => + connection.createQueryBuilder() + .insert() + .into(Post) + .values({ id, jsonField: () => ':field' } as any) // A bit of a hack to get the raw value inserting + .setParameter('field', value) + .execute() + + // This was the likely data within the database in #4440 + // This will happen if you've tried to manually insert the data in ways where + // we aren't expecting you to - like switching the column type to a text & + // trying to push a value into it that is an object. + await insert(1, "[object Object]"); + + const repo = connection.getRepository(Post); + + const getJson = async (id: number) => + ( + await repo.findOne({ where: { id } }) + )!.jsonField; + + await expect(getJson(1)).to.be.rejected; + }))); +});