Skip to content

Commit 81bac39

Browse files
pileroupilerou
authored
Remove read only and write only fields (#895)
* 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.
1 parent a9108a5 commit 81bac39

File tree

4 files changed

+353
-27
lines changed

4 files changed

+353
-27
lines changed

‎src/framework/ajv/index.ts

+38-24
Original file line numberDiff line numberDiff line change
@@ -117,19 +117,26 @@ function createAjv(
117117
compile: (sch, p, it) => {
118118
if (sch) {
119119
const validate: DataValidateFunction = (data, ctx) => {
120-
const isValid = data == null;
121-
if (!isValid) {
122-
validate.errors = [
123-
{
124-
keyword: 'readOnly',
125-
instancePath: ctx.instancePath,
126-
schemaPath: it.schemaPath.str,
127-
message: `is read-only`,
128-
params: { writeOnly: ctx.parentDataProperty },
129-
},
130-
];
120+
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
121+
// Remove readonly properties in request
122+
delete ctx.parentData[ctx.parentDataProperty];
123+
return true;
124+
}
125+
else {
126+
const isValid = data == null;
127+
if (!isValid) {
128+
validate.errors = [
129+
{
130+
keyword: 'readOnly',
131+
instancePath: ctx.instancePath,
132+
schemaPath: it.schemaPath.str,
133+
message: `is read-only`,
134+
params: { writeOnly: ctx.parentDataProperty },
135+
},
136+
];
137+
}
138+
return false;
131139
}
132-
return false;
133140
};
134141
return validate;
135142
}
@@ -178,19 +185,26 @@ function createAjv(
178185
compile: (sch, p, it) => {
179186
if (sch) {
180187
const validate: DataValidateFunction = (data, ctx) => {
181-
const isValid = data == null;
182-
if (!isValid) {
183-
validate.errors = [
184-
{
185-
keyword: 'writeOnly',
186-
instancePath: ctx.instancePath,
187-
schemaPath: it.schemaPath.str,
188-
message: `is write-only`,
189-
params: { writeOnly: ctx.parentDataProperty },
190-
},
191-
];
188+
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
189+
// Remove readonly properties in request
190+
delete ctx.parentData[ctx.parentDataProperty];
191+
return true;
192+
}
193+
else {
194+
const isValid = data == null;
195+
if (!isValid) {
196+
validate.errors = [
197+
{
198+
keyword: 'writeOnly',
199+
instancePath: ctx.instancePath,
200+
schemaPath: it.schemaPath.str,
201+
message: `is write-only`,
202+
params: {writeOnly: ctx.parentDataProperty},
203+
},
204+
];
205+
}
206+
return false;
192207
}
193-
return false;
194208
};
195209
return validate;
196210
}

‎test/read.only.removeadditional.spec.ts

+214
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
import * as path from 'path';
2+
import { expect } from 'chai';
3+
import * as request from 'supertest';
4+
import { createApp } from './common/app';
5+
import * as packageJson from '../package.json';
6+
7+
describe(packageJson.name, () => {
8+
let app = null;
9+
10+
before(async () => {
11+
// Set up the express app
12+
const apiSpec = path.join('test', 'resources', 'read.only.yaml');
13+
app = await createApp({ apiSpec, validateRequests: {removeAdditional:true}, validateResponses: true }, 3005, (app) =>
14+
app
15+
.post(`${app.basePath}/products`, (req, res) => res.json(req.body))
16+
.get(`${app.basePath}/products`, (req, res) =>
17+
res.json([
18+
{
19+
id: 'id_1',
20+
name: 'name_1',
21+
price: 9.99,
22+
created_at: new Date().toISOString(),
23+
},
24+
]),
25+
)
26+
.post(`${app.basePath}/products/inlined`, (req, res) =>
27+
res.json(req.body),
28+
)
29+
.post(`${app.basePath}/user`, (req, res) =>
30+
res.json({
31+
...req.body,
32+
...(req.query.include_id ? { id: 'test_id' } : {}),
33+
}),
34+
)
35+
.post(`${app.basePath}/user_inlined`, (req, res) =>
36+
res.json({
37+
...req.body,
38+
...(req.query.include_id ? { id: 'test_id' } : {}),
39+
}),
40+
)
41+
.post(`${app.basePath}/products/nested`, (req, res) => {
42+
const body = req.body;
43+
body.id = 'test';
44+
body.created_at = new Date().toISOString();
45+
body.reviews = body.reviews.map((r) => ({
46+
id: 99,
47+
rating: r.rating ?? 2,
48+
}));
49+
res.json(body);
50+
})
51+
.post(`${app.basePath}/readonly_required_allof`, (req, res) => {
52+
const json = {
53+
name: 'My Name',
54+
...(req.query.include_id ? { id: 'test_id' } : {}),
55+
};
56+
res.json(json);
57+
}),
58+
);
59+
});
60+
61+
after(() => {
62+
app.server.close();
63+
});
64+
65+
it('should remove read only properties in requests thanks to removeAdditional', async () =>
66+
request(app)
67+
.post(`${app.basePath}/products`)
68+
.set('content-type', 'application/json')
69+
.send({
70+
id: 'id_1',
71+
name: 'some name',
72+
price: 10.99,
73+
created_at: new Date().toISOString(),
74+
})
75+
.expect(200)
76+
.then((r) => {
77+
const body = r.body;
78+
// id is a readonly property and should not be allowed in the request
79+
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
80+
expect(body.id).to.be.undefined;
81+
}));
82+
83+
84+
it('should allow read only properties in responses', async () =>
85+
request(app)
86+
.get(`${app.basePath}/products`)
87+
.expect(200)
88+
.then((r) => {
89+
expect(r.body).to.be.an('array').with.length(1);
90+
}));
91+
92+
it('should remove read only inlined properties in requests thanks to removeAdditional', async () =>
93+
await request(app)
94+
.post(`${app.basePath}/products/inlined`)
95+
.set('content-type', 'application/json')
96+
.send({
97+
id: 'id_1',
98+
name: 'some name',
99+
price: 10.99,
100+
created_at: new Date().toISOString(),
101+
})
102+
.expect(200)
103+
.then((r) => {
104+
const body = r.body;
105+
// id is a readonly property and should not not be allowed in the request
106+
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
107+
expect(body.id).to.be.undefined;
108+
}));
109+
110+
111+
it('should remove read only properties in requests (nested and deep nested schema $refs) thanks to removeAdditional', async () =>
112+
request(app)
113+
.post(`${app.basePath}/products/nested`)
114+
.set('content-type', 'application/json')
115+
.send({
116+
id: 'id_1',
117+
name: 'some name',
118+
price: 10.99,
119+
created_at: new Date().toISOString(),
120+
reviews: [{
121+
id: 10,
122+
rating: 5,
123+
}],
124+
})
125+
.expect(200)
126+
.then((r) => {
127+
const body = r.body;
128+
// id is a readonly property and should not not be allowed in the request
129+
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
130+
expect(body.id).to.be.equal('test');
131+
expect(body.reviews[0].id).to.be.equal(99);
132+
}));
133+
134+
it('should pass validation if required read only properties to be missing from request ($ref)', async () =>
135+
request(app)
136+
.post(`${app.basePath}/user`)
137+
.set('content-type', 'application/json')
138+
.query({
139+
include_id: true,
140+
})
141+
.send({
142+
username: 'test',
143+
})
144+
.expect(200)
145+
.then((r) => {
146+
expect(r.body).to.be.an('object').with.property('id');
147+
expect(r.body).to.have.property('username');
148+
}));
149+
150+
it('should pass validation if required read only properties to be missing from request (inlined)', async () =>
151+
request(app)
152+
.post(`${app.basePath}/user_inlined`)
153+
.set('content-type', 'application/json')
154+
.query({
155+
include_id: true,
156+
})
157+
.send({
158+
username: 'test',
159+
})
160+
.expect(200)
161+
.then((r) => {
162+
expect(r.body).to.be.an('object').with.property('id');
163+
expect(r.body).to.have.property('username');
164+
}));
165+
166+
it('should pass validation if required read only properties to be missing from request (with charset)', async () =>
167+
request(app)
168+
.post(`${app.basePath}/user_inlined`)
169+
.set('content-type', 'application/json; charset=utf-8')
170+
.query({
171+
include_id: true,
172+
})
173+
.send({
174+
username: 'test',
175+
})
176+
.expect(200)
177+
.then((r) => {
178+
expect(r.body).to.be.an('object').with.property('id');
179+
expect(r.body).to.have.property('username');
180+
}));
181+
182+
it('should fail validation if required read only properties is missing from the response', async () =>
183+
request(app)
184+
.post(`${app.basePath}/user`)
185+
.set('content-type', 'application/json')
186+
.send({
187+
username: 'test',
188+
})
189+
.expect(500)
190+
.then((r) => {
191+
expect(r.body.errors[0])
192+
.to.have.property('message')
193+
.equals("must have required property 'id'");
194+
}));
195+
196+
it('should require readonly required property in response', async () =>
197+
request(app)
198+
.post(`${app.basePath}/readonly_required_allof`)
199+
.query({ include_id: true })
200+
.send({ optional: 'test' })
201+
.set('content-type', 'application/json')
202+
.expect(200));
203+
204+
it('should return 500 if readonly required property is missing from response', async () =>
205+
request(app)
206+
.post(`${app.basePath}/readonly_required_allof`)
207+
.query({ include_id: false })
208+
.send({ optional: 'test' })
209+
.set('content-type', 'application/json')
210+
.expect(500)
211+
.then((r) => {
212+
expect(r.body.message).includes("must have required property 'id'");
213+
}));
214+
});

‎test/read.only.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ describe(packageJson.name, () => {
9595
id: 'id_1',
9696
name: 'some name',
9797
price: 10.99,
98-
created_at: new Date().toUTCString(),
98+
created_at: new Date().toISOString(),
9999
})
100100
.expect(400)
101101
.then((r) => {
@@ -113,10 +113,10 @@ describe(packageJson.name, () => {
113113
name: 'some name',
114114
price: 10.99,
115115
created_at: new Date().toISOString(),
116-
reviews: {
116+
reviews: [{
117117
id: 'review_id',
118118
rating: 5,
119-
},
119+
}],
120120
})
121121
.expect(400)
122122
.then((r) => {

0 commit comments

Comments
 (0)