Skip to content

Commit

Permalink
Remove read only and write only fields (#895)
Browse files Browse the repository at this point in the history
* Fix problems in current test read.only according to the schema

* #627 Remove readonly fields in :
- requests if ``validateRequest.removeAdditional`` configuration equals ``true`` or ```'all'`` or ``'failing'``
- responses if ``validateResponse.removeAdditional`` configuration equals ``true`` or ```'all'`` or ``'failing'``
No changes if ``validateRequest = true``, ``validateResponse = true``, ``validateRequest.removeAdditional : false``, ``validateResponse.removeAdditional : false``

Unit tests added to check the behaviour with removeAdditional : true. Fields removed and no error in response.
  • Loading branch information
pilerou authored Jan 31, 2024
1 parent 631fb7b commit 97617fd
Show file tree
Hide file tree
Showing 4 changed files with 353 additions and 27 deletions.
62 changes: 38 additions & 24 deletions src/framework/ajv/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,19 +117,26 @@ function createAjv(
compile: (sch, p, it) => {
if (sch) {
const validate: DataValidateFunction = (data, ctx) => {
const isValid = data == null;
if (!isValid) {
validate.errors = [
{
keyword: 'readOnly',
instancePath: ctx.instancePath,
schemaPath: it.schemaPath.str,
message: `is read-only`,
params: { writeOnly: ctx.parentDataProperty },
},
];
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
// Remove readonly properties in request
delete ctx.parentData[ctx.parentDataProperty];
return true;
}
else {
const isValid = data == null;
if (!isValid) {
validate.errors = [
{
keyword: 'readOnly',
instancePath: ctx.instancePath,
schemaPath: it.schemaPath.str,
message: `is read-only`,
params: { writeOnly: ctx.parentDataProperty },
},
];
}
return false;
}
return false;
};
return validate;
}
Expand Down Expand Up @@ -178,19 +185,26 @@ function createAjv(
compile: (sch, p, it) => {
if (sch) {
const validate: DataValidateFunction = (data, ctx) => {
const isValid = data == null;
if (!isValid) {
validate.errors = [
{
keyword: 'writeOnly',
instancePath: ctx.instancePath,
schemaPath: it.schemaPath.str,
message: `is write-only`,
params: { writeOnly: ctx.parentDataProperty },
},
];
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
// Remove readonly properties in request
delete ctx.parentData[ctx.parentDataProperty];
return true;
}
else {
const isValid = data == null;
if (!isValid) {
validate.errors = [
{
keyword: 'writeOnly',
instancePath: ctx.instancePath,
schemaPath: it.schemaPath.str,
message: `is write-only`,
params: {writeOnly: ctx.parentDataProperty},
},
];
}
return false;
}
return false;
};
return validate;
}
Expand Down
214 changes: 214 additions & 0 deletions test/read.only.removeadditional.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
import * as path from 'path';
import { expect } from 'chai';
import * as request from 'supertest';
import { createApp } from './common/app';
import * as packageJson from '../package.json';

describe(packageJson.name, () => {
let app = null;

before(async () => {
// Set up the express app
const apiSpec = path.join('test', 'resources', 'read.only.yaml');
app = await createApp({ apiSpec, validateRequests: {removeAdditional:true}, validateResponses: true }, 3005, (app) =>
app
.post(`${app.basePath}/products`, (req, res) => res.json(req.body))
.get(`${app.basePath}/products`, (req, res) =>
res.json([
{
id: 'id_1',
name: 'name_1',
price: 9.99,
created_at: new Date().toISOString(),
},
]),
)
.post(`${app.basePath}/products/inlined`, (req, res) =>
res.json(req.body),
)
.post(`${app.basePath}/user`, (req, res) =>
res.json({
...req.body,
...(req.query.include_id ? { id: 'test_id' } : {}),
}),
)
.post(`${app.basePath}/user_inlined`, (req, res) =>
res.json({
...req.body,
...(req.query.include_id ? { id: 'test_id' } : {}),
}),
)
.post(`${app.basePath}/products/nested`, (req, res) => {
const body = req.body;
body.id = 'test';
body.created_at = new Date().toISOString();
body.reviews = body.reviews.map((r) => ({
id: 99,
rating: r.rating ?? 2,
}));
res.json(body);
})
.post(`${app.basePath}/readonly_required_allof`, (req, res) => {
const json = {
name: 'My Name',
...(req.query.include_id ? { id: 'test_id' } : {}),
};
res.json(json);
}),
);
});

after(() => {
app.server.close();
});

it('should remove read only properties in requests thanks to removeAdditional', async () =>
request(app)
.post(`${app.basePath}/products`)
.set('content-type', 'application/json')
.send({
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
})
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not be allowed in the request
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
expect(body.id).to.be.undefined;
}));


it('should allow read only properties in responses', async () =>
request(app)
.get(`${app.basePath}/products`)
.expect(200)
.then((r) => {
expect(r.body).to.be.an('array').with.length(1);
}));

it('should remove read only inlined properties in requests thanks to removeAdditional', async () =>
await request(app)
.post(`${app.basePath}/products/inlined`)
.set('content-type', 'application/json')
.send({
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
})
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not not be allowed in the request
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
expect(body.id).to.be.undefined;
}));


it('should remove read only properties in requests (nested and deep nested schema $refs) thanks to removeAdditional', async () =>
request(app)
.post(`${app.basePath}/products/nested`)
.set('content-type', 'application/json')
.send({
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
reviews: [{
id: 10,
rating: 5,
}],
})
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not not be allowed in the request
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
expect(body.id).to.be.equal('test');
expect(body.reviews[0].id).to.be.equal(99);
}));

it('should pass validation if required read only properties to be missing from request ($ref)', async () =>
request(app)
.post(`${app.basePath}/user`)
.set('content-type', 'application/json')
.query({
include_id: true,
})
.send({
username: 'test',
})
.expect(200)
.then((r) => {
expect(r.body).to.be.an('object').with.property('id');
expect(r.body).to.have.property('username');
}));

it('should pass validation if required read only properties to be missing from request (inlined)', async () =>
request(app)
.post(`${app.basePath}/user_inlined`)
.set('content-type', 'application/json')
.query({
include_id: true,
})
.send({
username: 'test',
})
.expect(200)
.then((r) => {
expect(r.body).to.be.an('object').with.property('id');
expect(r.body).to.have.property('username');
}));

it('should pass validation if required read only properties to be missing from request (with charset)', async () =>
request(app)
.post(`${app.basePath}/user_inlined`)
.set('content-type', 'application/json; charset=utf-8')
.query({
include_id: true,
})
.send({
username: 'test',
})
.expect(200)
.then((r) => {
expect(r.body).to.be.an('object').with.property('id');
expect(r.body).to.have.property('username');
}));

it('should fail validation if required read only properties is missing from the response', async () =>
request(app)
.post(`${app.basePath}/user`)
.set('content-type', 'application/json')
.send({
username: 'test',
})
.expect(500)
.then((r) => {
expect(r.body.errors[0])
.to.have.property('message')
.equals("must have required property 'id'");
}));

it('should require readonly required property in response', async () =>
request(app)
.post(`${app.basePath}/readonly_required_allof`)
.query({ include_id: true })
.send({ optional: 'test' })
.set('content-type', 'application/json')
.expect(200));

it('should return 500 if readonly required property is missing from response', async () =>
request(app)
.post(`${app.basePath}/readonly_required_allof`)
.query({ include_id: false })
.send({ optional: 'test' })
.set('content-type', 'application/json')
.expect(500)
.then((r) => {
expect(r.body.message).includes("must have required property 'id'");
}));
});
6 changes: 3 additions & 3 deletions test/read.only.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe(packageJson.name, () => {
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toUTCString(),
created_at: new Date().toISOString(),
})
.expect(400)
.then((r) => {
Expand All @@ -113,10 +113,10 @@ describe(packageJson.name, () => {
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
reviews: {
reviews: [{
id: 'review_id',
rating: 5,
},
}],
})
.expect(400)
.then((r) => {
Expand Down
Loading

0 comments on commit 97617fd

Please sign in to comment.