Skip to content

fix: array of objects with properties #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 15, 2019
Merged

fix: array of objects with properties #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 21 additions & 0 deletions src/__fixtures__/array-of-objects.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"type": "object",
"xml": {
"name": "Pet"
},
"properties": {
"propertyIsArrayOfObjects": {
"type": [
"array"
],
"items": {
"type": "object",
"properties": {
"ArrayObjectProperty": {
"type": "string"
}
}
}
}
}
}
77 changes: 77 additions & 0 deletions src/utils/__tests__/__snapshots__/renderSchema.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,82 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`renderSchema util should match array-of-objects.json 1`] = `
Array [
Object {
"canHaveChildren": true,
"id": "random-id",
"level": 0,
"metadata": Object {
"additionalProperties": undefined,
"annotations": Object {},
"enum": undefined,
"id": "random-id",
"path": Array [],
"patternProperties": undefined,
"properties": Object {
"propertyIsArrayOfObjects": Object {
"items": Object {
"properties": Object {
"ArrayObjectProperty": Object {
"type": "string",
},
},
"type": "object",
},
"type": Array [
"array",
],
},
},
"type": "object",
"validations": Object {},
},
"name": "",
},
Object {
"id": "random-id",
"level": 1,
"metadata": Object {
"additionalItems": undefined,
"annotations": Object {},
"enum": undefined,
"id": "random-id",
"items": undefined,
"name": "propertyIsArrayOfObjects",
"path": Array [
"properties",
"propertyIsArrayOfObjects",
],
"required": false,
"subtype": "object",
"type": "array",
"validations": Object {},
},
"name": "",
},
Object {
"id": "random-id",
"level": 3,
"metadata": Object {
"annotations": Object {},
"enum": undefined,
"id": "random-id",
"name": "ArrayObjectProperty",
"path": Array [
"properties",
"propertyIsArrayOfObjects",
"items",
"ArrayObjectProperty",
],
"required": false,
"type": "string",
"validations": Object {},
},
"name": "",
},
]
`;

exports[`renderSchema util should match combiner-schema.json 1`] = `
Array [
Object {
Expand Down
26 changes: 14 additions & 12 deletions src/utils/__tests__/renderSchema.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@ jest.mock('../assignId', () => ({
}));

describe('renderSchema util', () => {
it.each([['default-schema.json', ''], ['ref/original.json', 'ref/resolved.json'], ['combiner-schema.json', '']])(
'should match %s',
(schema, dereferenced) => {
expect(
Array.from(
renderSchema(
JSON.parse(fs.readFileSync(path.resolve(BASE_PATH, schema), 'utf-8')),
dereferenced ? JSON.parse(fs.readFileSync(path.resolve(BASE_PATH, dereferenced), 'utf-8')) : undefined
)
it.each([
['default-schema.json', ''],
['ref/original.json', 'ref/resolved.json'],
['combiner-schema.json', ''],
['array-of-objects.json', ''],
])('should match %s', (schema, dereferenced) => {
expect(
Array.from(
renderSchema(
JSON.parse(fs.readFileSync(path.resolve(BASE_PATH, schema), 'utf-8')),
dereferenced ? JSON.parse(fs.readFileSync(path.resolve(BASE_PATH, dereferenced), 'utf-8')) : undefined
)
).toMatchSnapshot();
}
);
)
).toMatchSnapshot();
});
});
24 changes: 14 additions & 10 deletions src/utils/renderSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,22 +103,26 @@ export const renderSchema: Walker = function*(schema, dereferencedSchema, level
(node as IArrayNode).additionalItems && { additional: (node as IArrayNode).additionalItems }),
},
} as SchemaTreeListNode;

if (Array.isArray(schema.items)) {
for (const [i, property] of schema.items.entries()) {
yield* renderSchema(property, dereferencedSchema, level + 1, {
path: [...path, 'items', i],
});
}
} else if (meta.subtype === 'object' && schema.items) {
yield* getProperties(schema.items, dereferencedSchema, level + 1, {
...meta,
path: [...path, 'items'],
});
} else if (meta.subtype === 'array' && schema.items) {
yield* renderSchema(schema.items, dereferencedSchema, level + 1, {
path,
});
} else if (schema.items) {
switch (baseNode.metadata && baseNode.metadata.subtype) {
case SchemaKind.Object:
yield* getProperties(schema.items, dereferencedSchema, level + 1, {
...meta,
path: [...path, 'items'],
});
break;
case SchemaKind.Array:
yield* renderSchema(schema.items, dereferencedSchema, level + 1, {
path,
});
break;
}
}
} else if ('properties' in node) {
// special case :P, it's
Expand Down
16 changes: 12 additions & 4 deletions src/utils/walk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const getCombiner = (node: JSONSchema4): JSONSchema4CombinerName | void => {
};

function assignNodeSpecificFields(base: IBaseNode, node: JSONSchema4) {
switch (node.type) {
switch (getType(node)) {
case SchemaKind.Array:
(base as IArrayNode).items = node.array;
(base as IArrayNode).additionalItems = node.additionalItems;
Expand All @@ -33,20 +33,28 @@ function assignNodeSpecificFields(base: IBaseNode, node: JSONSchema4) {
}
}

function getType(node: JSONSchema4) {
if (Array.isArray(node.type)) {
return node.type.length === 1 ? node.type[0] : node.type;
Copy link
Contributor

@lottamus lottamus May 9, 2019

Choose a reason for hiding this comment

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

We return a string if there's only one, but an array if theres none or more than one? Is that correct? Just making sure

Copy link
Contributor Author

@P0lip P0lip May 13, 2019

Choose a reason for hiding this comment

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

We return a string if there's only one, but an array if theres none or more than one? Is that correct? Just making sure

Well, I'm not sure how to handle such a scenario. For instance, there is no way we can display both array of items and object with properties.
Our UI is just too simple.

We would need to have something more or less similar to http://jlblcc.github.io/json-schema-viewer/ to handle such a case.

@marbemac what your thought on that?

Copy link
Contributor Author

@P0lip P0lip May 13, 2019

Choose a reason for hiding this comment

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

Also, we should be handling not, which we don't at the moment.
Any suggestions on the UI?

Copy link
Contributor

@lottamus lottamus May 13, 2019

Choose a reason for hiding this comment

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

there is no way we can display both array of items and object with properties

Is this what you mean?

image

Or something like oneOf?

image

Copy link
Contributor

@marbemac marbemac May 13, 2019

Choose a reason for hiding this comment

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

What is not?

We don't have to handle array and object in same property. We can suggest people use oneOf if they need that, although its kind of crazy and definitely an edge case.

Copy link
Contributor Author

@P0lip P0lip May 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

https://json-schema.org/understanding-json-schema/reference/combining.html#not
https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.6

Copy link
Contributor Author

@P0lip P0lip May 13, 2019

Choose a reason for hiding this comment

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

@lottamus yeah, pretty much that

{
  "type": ["array", "object"],
  "properties": {
    "bar": { 
      "type": "string" 
    }
  },
  "items": [
     {
       "type": "number"
     },
     {
       "type": "object",
       "properties": {
          "foo": { 
            "type": "number" 
          }
        }
     }
  ]
}

Copy link
Contributor

@marbemac marbemac May 13, 2019

Choose a reason for hiding this comment

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

Got it, we can skip not for now - I haven't seen it used in the wild by our customers yet, and would need to support it in JSE as well. A good addition for later, but not high priority right now.

}

return node.type;
}

function processNode(node: JSONSchema4): SchemaNode | void {
const combiner = getCombiner(node);

if (node.type !== undefined && combiner === undefined) {
const base: IBaseNode = {
id: assignId(node),
type: node.type,
type: getType(node),
validations: getValidations(node),
annotations: getAnnotations(node),
enum: node.enum,
};

if (Array.isArray(node.type)) {
if (node.type.includes('object')) {
if (Array.isArray(base.type)) {
if (base.type.includes('object')) {
// special case :P
assignNodeSpecificFields(base, {
...node,
Expand Down