Skip to content
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.

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

feat: Validate data and id #30

Merged
merged 4 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
// Use IntelliSense to learn about possible attributes.
// Hover to view descriptions of existing attributes.
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{
"type": "node",
"request": "launch",
"name": "Jasmine Tests",
"program": "${workspaceFolder:rpde-validator}/node_modules/jasmine/bin/jasmine.js",
"env": {
"NODE_PATH": "."
}
}
]
}
42 changes: 38 additions & 4 deletions src/rules/page/required-property-values-rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,18 @@ const RequiredPropertyValuesRule = class extends RpdeRule {
description: 'Validates that the feed properties conform to the correct format',
tests: {
state: {
description: 'Raises a failure if the state property isn\'t one of "updated" or "deleted"',
message: 'The [`state` property](https://www.openactive.io/realtime-paged-data-exchange/#-item-) must be of value "updated" or "deleted", but was found to be different in {{count}} items in the feed.',
description: 'Raises a failure if the `state` property isn\'t one of "updated" or "deleted"',
message: 'The [`state` property](https://www.openactive.io/realtime-paged-data-exchange/#-state-) must be of value "updated" or "deleted", but was found to be different in {{count}} items in the feed.',
sampleValues: {
count: 23,
},
category: ValidationErrorCategory.CONFORMANCE,
severity: ValidationErrorSeverity.FAILURE,
type: RpdeErrorType.INVALID_FORMAT,
},
id: {
description: 'Raises a failure if the `id` property isn\'t a string or an integer in any item',
message: 'The [`id` property](https://www.openactive.io/realtime-paged-data-exchange/#-id-) must be a string, but was found to be different in {{count}} items in the feed.',
sampleValues: {
count: 23,
},
Expand All @@ -23,8 +33,18 @@ const RequiredPropertyValuesRule = class extends RpdeRule {
type: RpdeErrorType.INVALID_FORMAT,
},
kind: {
description: 'Raises a failure if the kind property isn\'t a string in any item',
message: 'The [`kind` property](https://www.openactive.io/realtime-paged-data-exchange/#-item-) must be a string, but was found to be different in {{count}} items in the feed.',
description: 'Raises a failure if the `kind` property isn\'t a string in any item',
message: 'The [`kind` property](https://www.openactive.io/realtime-paged-data-exchange/#-kind-) must be a string, but was found to be different in {{count}} items in the feed.',
sampleValues: {
count: 23,
},
category: ValidationErrorCategory.CONFORMANCE,
severity: ValidationErrorSeverity.FAILURE,
type: RpdeErrorType.INVALID_FORMAT,
},
data: {
description: 'Raises a failure if the `data` property isn\'t an object in any item',
message: 'The [`data` property](https://www.openactive.io/realtime-paged-data-exchange/#-data-) must be an object, but was found to be different in {{count}} items in the feed.',
sampleValues: {
count: 23,
},
Expand Down Expand Up @@ -114,6 +134,8 @@ const RequiredPropertyValuesRule = class extends RpdeRule {
state: 0,
kind: 0,
item: 0,
id: 0,
data: 0,
};

if (node.data.items instanceof Array) {
Expand Down Expand Up @@ -142,6 +164,18 @@ const RequiredPropertyValuesRule = class extends RpdeRule {
) {
invalidProps.kind += 1;
}
if (
typeof item.id !== 'undefined'
&& typeof item.id !== 'string' && typeof item.id !== 'number'
) {
invalidProps.id += 1;
}
if (
typeof item.data !== 'undefined'
&& (typeof item.data !== 'object' || item.data instanceof Array)

Choose a reason for hiding this comment

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

Not sure if this is caught by other rules, but this allows for null, which has type object. It could in theory also have other clashes like with Dates, etc, though these are not possible at present as the data comes from JSON parsing.

I would suggest using lodash's isPlainObject function here to be more clear and to be more ironclad

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch @lukehesluke will push a new update that includes this

) {
invalidProps.data += 1;
}
}
}

Expand Down
56 changes: 55 additions & 1 deletion src/rules/page/required-property-values-rule.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,60 @@ describe('RequiredPropertyValuesRule', () => {
}
});

it('should raise an error when the id field is not a string or integer', () => {
json.items[0].id = [];
const node = new RpdeNode(
url,
json,
log,
);

rule.validate(node);

expect(log.addPageError).toHaveBeenCalled();
expect(log.pages.length).toBe(1);
expect(log.pages[0].errors.length).toBe(1);
for (const error of log.pages[0].errors) {
expect(error.type).toBe(RpdeErrorType.INVALID_FORMAT);
}
});

it('should raise an error when the data field is not an object', () => {
json.items[0].data = [];
const node = new RpdeNode(
url,
json,
log,
);

rule.validate(node);

expect(log.addPageError).toHaveBeenCalled();
expect(log.pages.length).toBe(1);
expect(log.pages[0].errors.length).toBe(1);
for (const error of log.pages[0].errors) {
expect(error.type).toBe(RpdeErrorType.INVALID_FORMAT);
}
});

it('should raise an error when the kind field is not a string', () => {
json.items[0].kind = 123;
const node = new RpdeNode(
url,
json,
log,
);

rule.validate(node);

expect(log.addPageError).toHaveBeenCalled();
expect(log.pages.length).toBe(1);
expect(log.pages[0].errors.length).toBe(1);
for (const error of log.pages[0].errors) {
expect(error.type).toBe(RpdeErrorType.INVALID_FORMAT);
}
});

it('should raise an error when the items field is not an array', () => {
json.items = {};
const node = new RpdeNode(
Expand All @@ -82,7 +136,7 @@ describe('RequiredPropertyValuesRule', () => {
}
});

it('should raise an error when there is an item field that is not an object', () => {
it('should raise an error when there is an items field that is not an object', () => {
json.items[0] = 1234;
const node = new RpdeNode(
url,
Expand Down
Loading