Skip to content
This repository was archived by the owner on Sep 7, 2020. It is now read-only.

Fix #489: Json Validator #513

Open
GaelGRIFFON wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix #489: Json Validator #513

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b13ec57
Json validator test
GaelGRIFFON Apr 12, 2018
b85e7de
Json validator
GaelGRIFFON Apr 12, 2018
f8bef30
JSON field is always valid
GaelGRIFFON May 21, 2018
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
20 changes: 20 additions & 0 deletions cli/packages/graphcool-cli-core/src/commands/import/Validator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,26 @@ describe('Validator', () => {
}),
).toBe(true)
})

test('Json', () => {
const types = `
type Post {
id: ID!
json: Json!
}
`
const validator = new Validator(types)
expect(() =>
validator.validateNode({ _typeName: 'Post', id: '25', Json: '' }),
).toThrow()
expect(
validator.validateNode({
_typeName: 'Post',
id: '25',
date: '{"test": "json"}',
}),
).toBe(true)
})

test('Int', () => {
const types = `
Expand Down
8 changes: 8 additions & 0 deletions cli/packages/graphcool-cli-core/src/commands/import/Validator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ export class Validator {
)
)
},
Json: (str) => {
Copy link

@adrienbarreau adrienbarreau Apr 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting an object in str so JSON.parse will always failed

Copy link

@adrienbarreau adrienbarreau Apr 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Json: (str) => {
      if(typeof str === 'string') {
          try {
              JSON.parse(str);
          } catch (e) {
              return false;
          }
          return true;
      }
      return true;
    },

Copy link
Author

@GaelGRIFFON GaelGRIFFON Apr 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @adrienbarreau

I don't understand why you get an object in str, you should get a string on Json format.

If you have an object, the test should fail...
Did I miss something?

Copy link

@mcmar mcmar May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaelGRIFFON Your code is assuming that the input to the Json function will be a string, but actually, it's an object. That's because the exporter already exports it as a javascript object.
The exporter serializes everything as a json file and Json fields "could" be serialized as a string in Json format, but they're not. They're serialized as a plain old javascript object. Or null. Or undefined. Or 0. Or literally any serializable js type. We already know that it's valid json because the original 000001.json file was successfully parsed. So at the time the import validator is running, this could be any type and it would be "valid json". Give that, here's my proposed solution:

Json: () => true,

Copy link
Author

@GaelGRIFFON GaelGRIFFON May 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK got it!
Thx you

try {
JSON.parse(str);
} catch (e) {
return false;
}
return true;
},
Boolean: isBoolean,
}
constructor(typesString: string) {
Expand Down